fix(cover): rebuild book thumbnails that went missing beside a cover (#1482)

A cover refresh skipped thumbnail generation whenever the stored cover
source was custom, so re-extract reported success while /thumbnail kept
404ing. Generate the missing thumbnail from the custom cover, and repair
any other gap lazily on the first thumbnail request.

Fixes #1475
This commit is contained in:
neon
2026-09-21 14:15:14 -06:00
committed by GitHub
parent 3bdd25e968
commit 970b152529
14 changed files with 546 additions and 83 deletions
@@ -3,7 +3,6 @@ import { join } from 'path';
import {
bookCoverDirPath,
bookThumbnailPath,
findExtractedBookCoverFileName,
findPreferredBookCoverFileName,
isCustomBookCoverFileName,
isExtractedBookCoverFileName,
@@ -29,9 +28,4 @@ describe('book-cover-storage', () => {
expect(findPreferredBookCoverFileName(['thumbnail.jpg', 'cover.png'])).toBe('cover.png');
expect(findPreferredBookCoverFileName(['thumbnail.jpg'])).toBeNull();
});
it('finds extracted cover files when present', () => {
expect(findExtractedBookCoverFileName(['cover_custom.png', 'cover_extracted.jpg'])).toBe('cover_extracted.jpg');
expect(findExtractedBookCoverFileName(['cover_custom.png'])).toBeNull();
});
});
-4
View File
@@ -25,10 +25,6 @@ export function findPreferredBookCoverFileName(files: readonly string[]): string
return files.find(isCustomBookCoverFileName) ?? files.find(isExtractedBookCoverFileName) ?? files.find(isLegacyBookCoverFileName) ?? null;
}
export function findExtractedBookCoverFileName(files: readonly string[]): string | null {
return files.find(isExtractedBookCoverFileName) ?? null;
}
function isLegacyBookCoverFileName(fileName: string): boolean {
return fileName.startsWith(COVER_LEGACY_FILE_PREFIX);
}
+14 -1
View File
@@ -172,6 +172,7 @@ function makeService(overrides: { bookMetadataLockService?: unknown } = {}) {
emitAuthorsReplaced: vi.fn(),
downloadAndSaveCover: vi.fn().mockResolvedValue(undefined),
refreshCoverForBook: vi.fn(),
ensureThumbnailForBook: vi.fn().mockResolvedValue(null),
};
const pipeline = {
run: vi.fn(),
@@ -853,7 +854,7 @@ describe('BookService', () => {
});
it('returns thumbnail path only when file is accessible', async () => {
const { service, bookRepo } = makeService();
const { service, bookRepo, metadataService } = makeService();
bookRepo.findLibraryIdByBookId.mockResolvedValue(5);
mockAccess.mockResolvedValue(undefined);
@@ -861,6 +862,18 @@ describe('BookService', () => {
mockAccess.mockRejectedValue(Object.assign(new Error('missing'), { code: 'ENOENT' }));
await expect(service.getThumbnailPath(9, makeUser())).resolves.toBeNull();
expect(metadataService.ensureThumbnailForBook).toHaveBeenCalledWith(9);
});
it('repairs and returns a missing thumbnail from the active cover', async () => {
const { service, bookRepo, metadataService } = makeService();
bookRepo.findLibraryIdByBookId.mockResolvedValue(5);
mockAccess.mockRejectedValue(Object.assign(new Error('missing'), { code: 'ENOENT' }));
metadataService.ensureThumbnailForBook.mockResolvedValue('/tmp/books/covers/9/thumbnail.jpg');
await expect(service.getThumbnailPath(9, makeUser())).resolves.toBe('/tmp/books/covers/9/thumbnail.jpg');
expect(metadataService.ensureThumbnailForBook).toHaveBeenCalledWith(9);
});
it('throws when thumbnail access fails for non-missing errors', async () => {
+1 -1
View File
@@ -1315,7 +1315,7 @@ export class BookService {
await access(path);
return path;
} catch (err) {
if (this.isMissingFilesystemEntry(err)) return null;
if (this.isMissingFilesystemEntry(err)) return this.metadataService.ensureThumbnailForBook(id);
const errorClass = err instanceof Error ? err.name : 'Error';
const errorMessage = sanitizeLogValue(err instanceof Error ? err.message : String(err));
const pathValue = sanitizeLogValue(path);
+11 -6
View File
@@ -7,7 +7,7 @@ import { NodePgDatabase } from 'drizzle-orm/node-postgres';
import { mkdir, readdir, readFile, rename, unlink, writeFile } from 'fs/promises';
import { basename, join } from 'path';
import { bookCoverDirPath, bookThumbnailPath, findExtractedBookCoverFileName } from '../../common/book-cover-storage';
import { bookCoverDirPath, bookThumbnailPath, findPreferredBookCoverFileName, isCustomBookCoverFileName } from '../../common/book-cover-storage';
import type { RequestUser } from '../../common/types/request-user';
import { sanitizeLogValue } from '../../common/utils/log-sanitize.utils';
import { ensureSafeRemoteHost } from '../../common/utils/ssrf.utils';
@@ -167,8 +167,8 @@ export class CoverService {
const dir = bookCoverDirPath(this.appDataPath, bookId);
await this.deleteFilesByPrefix(dir, COVER_CUSTOM_FILE_PREFIX);
const extractedPath = await this.findExtractedCover(bookId);
if (!extractedPath) {
const fallbackPath = await this.findFallbackCover(bookId);
if (!fallbackPath) {
await this.removeFileIfPresent(bookThumbnailPath(this.appDataPath, bookId));
await this.setCoverSource(bookId, null);
this.logger.log(
@@ -177,7 +177,7 @@ export class CoverService {
return null;
}
const bytes = await readFile(extractedPath);
const bytes = await readFile(fallbackPath);
const thumb = await generateThumbnail(bytes);
await writeFile(bookThumbnailPath(this.appDataPath, bookId), thumb);
await this.setCoverSource(bookId, 'extracted');
@@ -281,10 +281,15 @@ export class CoverService {
}
}
private async findExtractedCover(bookId: number): Promise<string | null> {
/**
* Resolves the cover left behind once the custom cover is deleted. This must use the same
* preference order the cover endpoint serves from, including legacy `cover.*` files: matching on
* extracted covers alone dropped the thumbnail for a book the endpoint still served (issue #1475).
*/
private async findFallbackCover(bookId: number): Promise<string | null> {
const dir = bookCoverDirPath(this.appDataPath, bookId);
const files = await this.readDirIfExists(dir);
const found = findExtractedBookCoverFileName(files);
const found = findPreferredBookCoverFileName(files.filter((fileName) => !isCustomBookCoverFileName(fileName)));
return found ? join(dir, found) : null;
}
@@ -145,6 +145,7 @@ function makeService(
const bookService = {
getDetail: vi.fn().mockResolvedValue(makeDetail()),
verifyBookAccess: vi.fn().mockResolvedValue(undefined),
getThumbnailPath: vi.fn().mockResolvedValue(join('/data', 'covers', '10', 'thumbnail.jpg')),
bulkSetRating: vi.fn().mockResolvedValue(undefined),
verifyFileAccess: vi.fn().mockResolvedValue({
id: 100,
@@ -274,7 +275,6 @@ function makeService(
getDeviceFileNamingPattern: vi.fn().mockResolvedValue(deviceOrganization),
} as never,
pluginService as never,
{ appDataPath: '/data', bookDockPath: '/data/book-dock' },
);
return {
@@ -852,7 +852,7 @@ describe('KoreaderCatalogService', () => {
await service.streamThumbnail(makeUser(), 10, reply as never);
expect(bookService.verifyBookAccess).toHaveBeenCalledWith(10, expect.objectContaining({ id: 1 }));
expect(bookService.getThumbnailPath).toHaveBeenCalledWith(10, expect.objectContaining({ id: 1 }));
expect(reply.header).toHaveBeenCalledWith('ETag', '"5000"');
expect(reply.type).toHaveBeenCalledWith('image/jpeg');
expect(mockCreateReadStream).toHaveBeenCalledWith(join('/data', 'covers', '10', 'thumbnail.jpg'));
@@ -909,6 +909,9 @@ describe('KoreaderCatalogService', () => {
mockStat.mockRejectedValueOnce(new Error('missing'));
await expect(service.streamThumbnail(makeUser(), 10, makeReply() as never)).rejects.toThrow(NotFoundException);
bookService.getThumbnailPath.mockResolvedValueOnce(null);
await expect(service.streamThumbnail(makeUser(), 10, makeReply() as never)).rejects.toThrow(NotFoundException);
});
it('paginates author sections with a filter and navigation links', async () => {
@@ -3,8 +3,7 @@ import { createReadStream } from 'fs';
import { stat } from 'fs/promises';
import { basename } from 'path';
import { BadRequestException, Inject, Injectable, InternalServerErrorException, Logger, NotFoundException } from '@nestjs/common';
import type { ConfigType } from '@nestjs/config';
import { BadRequestException, Injectable, InternalServerErrorException, Logger, NotFoundException } from '@nestjs/common';
import type { FastifyReply } from 'fastify';
import { DEFAULT_KOREADER_DEVICE_PATTERN, resolveUploadPath } from '@bookorbit/types';
@@ -36,13 +35,11 @@ import type {
KoreaderCatalogSort,
KoreaderCatalogSortOrder,
} from '@bookorbit/types';
import { bookThumbnailPath } from '../../common/book-cover-storage';
import { MAX_OFFSET_ROWS, isOffsetWithinLimit } from '../../common/constants/pagination.constants';
import { imageContentTypeFromPath } from '../../common/image-content-type';
import type { RequestUser } from '../../common/types/request-user';
import { contentDispositionHeader } from '../../common/utils/content-disposition.utils';
import { sanitizeLogValue } from '../../common/utils/log-sanitize.utils';
import { storageConfig } from '../../config/config';
import { BookReadService } from '../book/book-read.service';
import { BookService } from '../book/book.service';
import { BrowseCountsService } from '../browse-counts/browse-counts.service';
@@ -151,7 +148,6 @@ export class KoreaderCatalogService {
private readonly appSettingsService: AppSettingsService,
private readonly koreaderService: KoreaderService,
private readonly pluginService: KoreaderPluginService,
@Inject(storageConfig.KEY) private readonly storage: ConfigType<typeof storageConfig>,
) {}
getRoot(): { sections: KoreaderCatalogEntry[] } {
@@ -481,8 +477,8 @@ export class KoreaderCatalogService {
}
async streamThumbnail(user: RequestUser, bookId: number, reply: FastifyReply, ifNoneMatch?: string): Promise<void> {
await this.bookService.verifyBookAccess(bookId, user);
const thumbnailPath = bookThumbnailPath(this.storage.appDataPath, bookId);
const thumbnailPath = await this.bookService.getThumbnailPath(bookId, user);
if (!thumbnailPath) throw new NotFoundException('No thumbnail');
try {
const { mtimeMs } = await stat(thumbnailPath);
const etag = `"${Math.floor(mtimeMs)}"`;
@@ -0,0 +1,104 @@
import { ConfigService } from '@nestjs/config';
import { Test } from '@nestjs/testing';
import { access, mkdir, mkdtemp, readFile, readdir, rm, writeFile } from 'fs/promises';
import { tmpdir } from 'os';
import { join } from 'path';
import sharp from 'sharp';
import { DB } from '../../db';
import { BookMetadataLockService } from '../book-metadata-lock/book-metadata-lock.service';
import { BookEmbedderService } from '../embedding/book-embedder.service';
import { MetadataScoreService } from '../metadata-score/metadata-score.service';
import { NarratorService } from '../narrator/narrator.service';
import { ComicMetadataRepository } from './comic-metadata.repository';
import { MetadataExtractionService } from './metadata-extraction.service';
import { MetadataService } from './metadata.service';
describe('MetadataService thumbnail repair', () => {
let appDataPath: string;
let service: MetadataService;
async function makeCover(): Promise<Buffer> {
return sharp({ create: { width: 800, height: 1200, channels: 3, background: '#336699' } })
.jpeg()
.toBuffer();
}
beforeEach(async () => {
appDataPath = await mkdtemp(join(tmpdir(), 'bookorbit-thumbnail-repair-'));
const moduleRef = await Test.createTestingModule({
providers: [
MetadataService,
{ provide: DB, useValue: {} },
{ provide: ConfigService, useValue: { get: () => appDataPath } },
{ provide: MetadataExtractionService, useValue: {} },
{ provide: MetadataScoreService, useValue: { calculateAndSave: vi.fn() } },
{ provide: NarratorService, useValue: {} },
{ provide: ComicMetadataRepository, useValue: {} },
{ provide: BookMetadataLockService, useValue: { isFieldLocked: vi.fn().mockResolvedValue(false) } },
{ provide: BookEmbedderService, useValue: {} },
],
}).compile();
service = moduleRef.get(MetadataService);
});
afterEach(async () => {
await rm(appDataPath, { recursive: true, force: true });
});
async function seedCoverDir(bookId: number, coverFileName: string, bytes: Buffer): Promise<string> {
const coverDir = join(appDataPath, 'covers', String(bookId));
await mkdir(coverDir, { recursive: true });
await writeFile(join(coverDir, coverFileName), bytes);
return coverDir;
}
it('reproduces and repairs a missing thumbnail beside an intact custom cover', async () => {
const coverDir = await seedCoverDir(913, 'cover_custom.jpg', await makeCover());
const thumbnailPath = join(coverDir, 'thumbnail.jpg');
await expect(access(thumbnailPath)).rejects.toMatchObject({ code: 'ENOENT' });
await expect(service.ensureThumbnailForBook(913)).resolves.toBe(thumbnailPath);
const metadata = await sharp(await readFile(thumbnailPath)).metadata();
expect(metadata).toMatchObject({ format: 'jpeg', width: 400, height: 600 });
});
it('repairs from a legacy cover file the cover endpoint still serves', async () => {
const coverDir = await seedCoverDir(914, 'cover.jpg', await makeCover());
await expect(service.ensureThumbnailForBook(914)).resolves.toBe(join(coverDir, 'thumbnail.jpg'));
});
it('leaves no temporary file behind once the thumbnail is published', async () => {
const coverDir = await seedCoverDir(915, 'cover_extracted.jpg', await makeCover());
await service.ensureThumbnailForBook(915);
await expect(readdir(coverDir)).resolves.toEqual(expect.arrayContaining(['cover_extracted.jpg', 'thumbnail.jpg']));
await expect(readdir(coverDir)).resolves.toHaveLength(2);
});
it('keeps a thumbnail written while the repair was in flight', async () => {
const coverDir = await seedCoverDir(916, 'cover_custom.jpg', await makeCover());
const thumbnailPath = join(coverDir, 'thumbnail.jpg');
const existing = Buffer.from('thumbnail written by a concurrent cover update');
const repair = service.ensureThumbnailForBook(916);
await writeFile(thumbnailPath, existing);
await expect(repair).resolves.toBe(thumbnailPath);
await expect(readFile(thumbnailPath)).resolves.toEqual(existing);
});
it('resolves null instead of throwing when the cover cannot be decoded', async () => {
await seedCoverDir(917, 'cover_custom.jpg', Buffer.alloc(0));
await expect(service.ensureThumbnailForBook(917)).resolves.toBeNull();
});
it('resolves null when the book has no cover directory at all', async () => {
await expect(service.ensureThumbnailForBook(918)).resolves.toBeNull();
});
});
@@ -1,6 +1,9 @@
vi.mock('fs/promises', () => ({
access: vi.fn(),
link: vi.fn(),
mkdir: vi.fn(),
readFile: vi.fn(),
rename: vi.fn(),
writeFile: vi.fn(),
readdir: vi.fn().mockResolvedValue([]),
rm: vi.fn(),
@@ -84,7 +87,7 @@ vi.mock('./extractors/audio.extractor', () => ({
probeAudioChapters: vi.fn().mockImplementation(() => Promise.resolve({ chapters: [], durationMs: null })),
}));
import { mkdir, readFile, readdir, rm, writeFile } from 'fs/promises';
import { access, link, mkdir, readFile, readdir, rename, rm, writeFile } from 'fs/promises';
import { Logger } from '@nestjs/common';
import { authors, bookAuthors, bookGenres, bookMetadata, books, bookTags, genres, tags } from '../../db/schema';
@@ -101,6 +104,9 @@ import { METADATA_AUTHORS_REPLACED } from './metadata-events.service';
import { MetadataService } from './metadata.service';
import { MetadataExtractionService } from './metadata-extraction.service';
const mockAccess = access as MockedFunction<typeof access>;
const mockLink = link as MockedFunction<typeof link>;
const mockRename = rename as MockedFunction<typeof rename>;
const mockMkdir = mkdir as MockedFunction<typeof mkdir>;
const mockReadFile = readFile as MockedFunction<typeof readFile>;
const mockWriteFile = writeFile as MockedFunction<typeof writeFile>;
@@ -173,6 +179,9 @@ describe('MetadataService', () => {
config.get.mockReturnValue('/books');
embedder.embedBook.mockResolvedValue(undefined);
mockAccess.mockRejectedValue(Object.assign(new Error('missing'), { code: 'ENOENT' }));
mockLink.mockResolvedValue(undefined);
mockRename.mockResolvedValue(undefined);
mockMkdir.mockResolvedValue(undefined);
mockReadFile.mockResolvedValue('');
mockWriteFile.mockResolvedValue(undefined);
@@ -242,6 +251,37 @@ describe('MetadataService', () => {
);
}
function stubEpubCoverExtraction(cover: Buffer): void {
mockExtractEpubMetadata.mockResolvedValueOnce({
title: 'Refreshable book',
subtitle: null,
description: null,
isbn10: null,
isbn13: null,
publisher: null,
publishedYear: null,
language: null,
seriesName: null,
seriesIndex: null,
authors: [],
narrators: [],
genres: [],
tags: [],
rating: null,
pageCount: null,
googleBooksId: null,
goodreadsId: null,
amazonId: null,
hardcoverId: null,
hardcoverEditionId: null,
openLibraryId: null,
ranobedbId: null,
itunesId: null,
coverBuffer: null,
});
mockExtractEpubCover.mockResolvedValueOnce(cover);
}
it('downloadAndSaveCover writes cover/thumbnail and updates metadata when download is valid', async () => {
const { db, updateSet } = makeDb();
const service = makeService(db);
@@ -275,37 +315,6 @@ describe('MetadataService', () => {
// The Kobo CoverImageId is versioned from coverUpdatedAt, so it must move whenever the served
// image does, and stay put when it does not (issue #943).
describe('coverUpdatedAt stamping', () => {
function stubEpubCoverExtraction(cover: Buffer): void {
mockExtractEpubMetadata.mockResolvedValueOnce({
title: 'Refreshable book',
subtitle: null,
description: null,
isbn10: null,
isbn13: null,
publisher: null,
publishedYear: null,
language: null,
seriesName: null,
seriesIndex: null,
authors: [],
narrators: [],
genres: [],
tags: [],
rating: null,
pageCount: null,
googleBooksId: null,
goodreadsId: null,
amazonId: null,
hardcoverId: null,
hardcoverEditionId: null,
openLibraryId: null,
ranobedbId: null,
itunesId: null,
coverBuffer: null,
});
mockExtractEpubCover.mockResolvedValueOnce(cover);
}
it('stamps coverUpdatedAt when an overwriting extraction replaces the cover', async () => {
const { db, updateSet } = makeDb();
const service = makeService(db);
@@ -399,6 +408,157 @@ describe('MetadataService', () => {
expect(db.update).not.toHaveBeenCalledWith(books);
});
it('refreshCoverForBook repairs a missing thumbnail from the active custom cover', async () => {
const { db, selectLimit, updateSet } = makeDb();
const service = makeService(db);
selectLimit.mockResolvedValue([{ coverSource: 'custom' }]);
mockReaddir.mockResolvedValue(['cover_custom.jpg', 'cover_extracted.png']);
mockReadFile.mockResolvedValue(Buffer.from('custom-cover-bytes'));
stubEpubCoverExtraction(Buffer.from('extracted-cover-bytes'));
await expect(service.refreshCoverForBook(14, '/book.epub', 'epub')).resolves.toBe(true);
expect(mockReadFile).toHaveBeenCalledWith('/books/covers/14/cover_custom.jpg');
expect(mockGenerateThumbnail).toHaveBeenCalledWith(Buffer.from('custom-cover-bytes'));
expect(mockWriteFile).toHaveBeenCalledWith('/books/covers/14/thumbnail.jpg', Buffer.from('thumbnail-bytes'));
expect(updateSet).toHaveBeenCalledWith({ coverUpdatedAt: expect.any(Date) });
expect(db.update).toHaveBeenCalledWith(books);
});
it('refreshCoverForBook promotes the extracted cover when the stored custom cover is missing', async () => {
const { db, selectLimit, updateSet } = makeDb();
const service = makeService(db);
selectLimit.mockResolvedValue([{ coverSource: 'custom' }]);
mockReaddir.mockResolvedValue(['cover_extracted.jpg']);
stubEpubCoverExtraction(Buffer.from('extracted-cover-bytes'));
await expect(service.refreshCoverForBook(16, '/book.epub', 'epub')).resolves.toBe(true);
expect(mockWriteFile).toHaveBeenCalledWith('/books/covers/16/thumbnail.jpg', Buffer.from('thumbnail-bytes'));
expect(updateSet).toHaveBeenCalledWith({ coverSource: 'extracted', updatedAt: expect.any(Date) });
});
it('refreshCoverForBook keeps the custom cover source when the cover directory reads back empty', async () => {
const { db, selectLimit, updateSet } = makeDb();
const service = makeService(db);
selectLimit.mockResolvedValue([{ coverSource: 'custom' }]);
// An unmounted cover volume is recreated by mkdir and then lists empty, which must not be read
// as "the user deleted their custom cover".
mockReaddir.mockResolvedValue([]);
stubEpubCoverExtraction(Buffer.from('extracted-cover-bytes'));
await expect(service.refreshCoverForBook(19, '/book.epub', 'epub')).resolves.toBe(true);
expect(updateSet).not.toHaveBeenCalledWith({ coverSource: 'extracted', updatedAt: expect.any(Date) });
});
it('ensureThumbnailForBook lazily repairs a missing thumbnail from the preferred cover', async () => {
const { db } = makeDb();
const service = makeService(db);
mockReaddir.mockResolvedValue(['cover_extracted.png', 'cover_custom.jpg']);
mockReadFile.mockResolvedValue(Buffer.from('custom-cover-bytes'));
await expect(service.ensureThumbnailForBook(15)).resolves.toBe('/books/covers/15/thumbnail.jpg');
expect(mockReadFile).toHaveBeenCalledWith('/books/covers/15/cover_custom.jpg');
expect(mockGenerateThumbnail).toHaveBeenCalledWith(Buffer.from('custom-cover-bytes'));
// The thumbnail is staged in the cover directory and hard-linked into place, so an interrupted
// write can never leave a truncated thumbnail.jpg that later requests would accept.
const [tempPath, tempBytes] = mockWriteFile.mock.calls[0]!;
expect(tempPath).toMatch(/^\/books\/covers\/15\/\.thumbnail-repair-.+\.tmp$/);
expect(tempBytes).toEqual(Buffer.from('thumbnail-bytes'));
expect(mockLink).toHaveBeenCalledWith(tempPath, '/books/covers/15/thumbnail.jpg');
expect(mockRm).toHaveBeenCalledWith(tempPath, { force: true });
});
it('ensureThumbnailForBook keeps a thumbnail created by a concurrent cover update', async () => {
const { db } = makeDb();
const service = makeService(db);
mockReaddir.mockResolvedValue(['cover_custom.jpg']);
mockReadFile.mockResolvedValue(Buffer.from('custom-cover-bytes'));
mockLink.mockRejectedValue(Object.assign(new Error('already exists'), { code: 'EEXIST' }));
await expect(service.ensureThumbnailForBook(17)).resolves.toBe('/books/covers/17/thumbnail.jpg');
expect(mockWriteFile).not.toHaveBeenCalledWith('/books/covers/17/thumbnail.jpg', expect.anything(), expect.anything());
});
it('ensureThumbnailForBook publishes by rename when the filesystem has no hard links', async () => {
const { db } = makeDb();
const service = makeService(db);
mockReaddir.mockResolvedValue(['cover_custom.jpg']);
mockReadFile.mockResolvedValue(Buffer.from('custom-cover-bytes'));
mockLink.mockRejectedValue(Object.assign(new Error('operation not permitted'), { code: 'EPERM' }));
await expect(service.ensureThumbnailForBook(26)).resolves.toBe('/books/covers/26/thumbnail.jpg');
// The bytes must reach the served path through a rename of a fully written file, never a direct
// write, so an interrupted repair cannot leave a truncated thumbnail that nothing revalidates.
const [tempPath] = mockWriteFile.mock.calls[0]!;
expect(tempPath).toMatch(/^\/books\/covers\/26\/\.thumbnail-repair-.+\.tmp$/);
expect(mockRename).toHaveBeenCalledWith(tempPath, '/books/covers/26/thumbnail.jpg');
expect(mockWriteFile).not.toHaveBeenCalledWith('/books/covers/26/thumbnail.jpg', expect.anything(), expect.anything());
});
it('ensureThumbnailForBook leaves a published thumbnail alone when hard links are unavailable', async () => {
const { db } = makeDb();
const service = makeService(db);
mockReaddir.mockResolvedValue(['cover_custom.jpg']);
mockReadFile.mockResolvedValue(Buffer.from('custom-cover-bytes'));
mockLink.mockRejectedValue(Object.assign(new Error('operation not permitted'), { code: 'EPERM' }));
mockAccess.mockResolvedValue(undefined);
await expect(service.ensureThumbnailForBook(27)).resolves.toBe('/books/covers/27/thumbnail.jpg');
expect(mockRename).not.toHaveBeenCalled();
});
it('ensureThumbnailForBook resolves null instead of throwing when the cover cannot be read', async () => {
const { db } = makeDb();
const service = makeService(db);
mockReaddir.mockResolvedValue(['cover_custom.jpg']);
mockReadFile.mockResolvedValue(Buffer.from('custom-cover-bytes'));
mockGenerateThumbnail.mockRejectedValueOnce(new Error('Input buffer contains unsupported image format'));
await expect(service.ensureThumbnailForBook(18)).resolves.toBeNull();
});
it('ensureThumbnailForBook bounds concurrent thumbnail generation', async () => {
const { db } = makeDb();
const service = makeService(db);
let active = 0;
let peak = 0;
const releases: Array<() => void> = [];
mockReaddir.mockResolvedValue(['cover_custom.jpg']);
mockReadFile.mockImplementation(
() =>
new Promise<Buffer>((resolve) => {
active++;
peak = Math.max(peak, active);
releases.push(() => {
active--;
resolve(Buffer.from('custom-cover-bytes'));
});
}),
);
const repairs = [21, 22, 23, 24, 25].map((bookId) => service.ensureThumbnailForBook(bookId));
await vi.waitFor(() => expect(releases).toHaveLength(4));
expect(peak).toBe(4);
releases.splice(0).forEach((release) => release());
await vi.waitFor(() => expect(releases).toHaveLength(1));
releases.splice(0).forEach((release) => release());
await expect(Promise.all(repairs)).resolves.toEqual([
'/books/covers/21/thumbnail.jpg',
'/books/covers/22/thumbnail.jpg',
'/books/covers/23/thumbnail.jpg',
'/books/covers/24/thumbnail.jpg',
'/books/covers/25/thumbnail.jpg',
]);
});
it('downloadAndSaveCover no-ops on empty payloads and network failures', async () => {
const { db } = makeDb();
const service = makeService(db);
+147 -7
View File
@@ -2,15 +2,18 @@ import { Inject, Injectable, Logger, Optional } from '@nestjs/common';
import { ConfigService } from '@nestjs/config';
import { and, eq, inArray, isNull, sql } from 'drizzle-orm';
import { NodePgDatabase } from 'drizzle-orm/node-postgres';
import { mkdir, readdir, rm, writeFile } from 'fs/promises';
import { randomUUID } from 'crypto';
import { access, link, mkdir, readFile, readdir, rename, rm, writeFile } from 'fs/promises';
import { join } from 'path';
import { DB } from '../../db';
import * as schema from '../../db/schema';
import {
COVER_EXTRACTED_FILE_PREFIX,
COVER_THUMBNAIL_FILE_NAME,
bookCoverDirPath,
bookThumbnailPath,
findPreferredBookCoverFileName,
isCustomBookCoverFileName,
isExtractedBookCoverFileName,
} from '../../common/book-cover-storage';
@@ -51,6 +54,7 @@ interface RelationMutationOptions {
const MAX_RELATION_NAME_LENGTH = 200;
const EXTRACTED_COVER_SOURCE = 'extracted';
const THUMBNAIL_REPAIR_CONCURRENCY = 4;
const MIN_PUBLISHED_YEAR = 1000;
const MAX_PUBLISHED_YEAR = 2200;
const NORMALIZED_AUTHOR_NAME_SQL = normalizeMetadataTextKeySql(authors.name);
@@ -67,6 +71,9 @@ function normalizePublishedYear(year: number | null | undefined): number | null
export class MetadataService {
private readonly logger = new Logger(MetadataService.name);
private readonly appDataPath: string;
private readonly thumbnailRepairs = new Map<number, Promise<string | null>>();
private readonly thumbnailRepairWaiters: Array<() => void> = [];
private activeThumbnailRepairs = 0;
constructor(
@Inject(DB) private readonly db: Db,
@@ -260,6 +267,24 @@ export class MetadataService {
return true;
}
/**
* Regenerates a thumbnail that went missing beside an intact cover (issue #1475). Resolves to
* null rather than throwing, so a cover that sharp cannot read stays a 404 placeholder in the
* grid instead of failing the request.
*/
async ensureThumbnailForBook(bookId: number): Promise<string | null> {
const existing = this.thumbnailRepairs.get(bookId);
if (existing) return existing;
const pending = this.withThumbnailRepairSlot(() => this.repairMissingThumbnail(bookId));
this.thumbnailRepairs.set(bookId, pending);
try {
return await pending;
} finally {
if (this.thumbnailRepairs.get(bookId) === pending) this.thumbnailRepairs.delete(bookId);
}
}
// ── Audio helpers ────────────────────────────────────────────────────────────
/**
@@ -859,7 +884,13 @@ export class MetadataService {
.from(bookMetadata)
.where(eq(bookMetadata.bookId, bookId))
.limit(1);
const preserveCustom = !overwrite && currentCover?.coverSource === 'custom';
const customCoverFileName = files.find(isCustomBookCoverFileName);
const storedCustomCoverMissing = !overwrite && currentCover?.coverSource === 'custom' && customCoverFileName === undefined;
const preserveCustom = !overwrite && currentCover?.coverSource === 'custom' && customCoverFileName !== undefined;
// An empty listing cannot tell a deleted custom cover apart from cover storage that failed to
// mount, and mkdir above recreates the directory either way. Only demote the stored source when
// some other artifact proves the directory is the real one.
const replaceMissingCustom = storedCustomCoverMissing && files.length > 0;
const staleCoverFiles = files.filter(
(fileName) => isExtractedBookCoverFileName(fileName) || (!preserveCustom && isCustomBookCoverFileName(fileName)),
@@ -868,14 +899,21 @@ export class MetadataService {
await writeFile(join(dir, `${COVER_EXTRACTED_FILE_PREFIX}${ext}`), bytes);
if (!preserveCustom) {
let thumbnailUpdated = false;
if (preserveCustom && customCoverFileName && !files.includes(COVER_THUMBNAIL_FILE_NAME)) {
const customCover = await readFile(join(dir, customCoverFileName));
const thumbnail = await generateThumbnail(customCover);
await writeFile(bookThumbnailPath(this.appDataPath, bookId), thumbnail);
thumbnailUpdated = true;
} else if (!preserveCustom) {
const thumbnail = await generateThumbnail(bytes);
await writeFile(bookThumbnailPath(this.appDataPath, bookId), thumbnail);
thumbnailUpdated = true;
}
const now = new Date();
if (overwrite) {
if (overwrite || replaceMissingCustom) {
await this.db.update(bookMetadata).set({ coverSource: EXTRACTED_COVER_SOURCE, updatedAt: now }).where(eq(bookMetadata.bookId, bookId));
} else {
await this.db
@@ -884,14 +922,116 @@ export class MetadataService {
.where(and(eq(bookMetadata.bookId, bookId), isNull(bookMetadata.coverSource)));
}
if (!preserveCustom) {
// The extracted cover and its thumbnail were just rewritten, so the served image moved even
// when the first-writer-wins update above matched no row.
if (thumbnailUpdated) {
// A served cover artifact was rewritten, so its cache version must move even when the
// first-writer-wins update above matched no row.
await this.db.update(bookMetadata).set({ coverUpdatedAt: now }).where(eq(bookMetadata.bookId, bookId));
await this.db.update(books).set({ updatedAt: now }).where(eq(books.id, bookId));
}
}
private async repairMissingThumbnail(bookId: number): Promise<string | null> {
const event = 'metadata.thumbnail_repair';
const startedAt = Date.now();
const dir = bookCoverDirPath(this.appDataPath, bookId);
const thumbnailPath = bookThumbnailPath(this.appDataPath, bookId);
try {
const files = await readdir(dir);
if (files.includes(COVER_THUMBNAIL_FILE_NAME)) return thumbnailPath;
const coverFileName = findPreferredBookCoverFileName(files);
if (!coverFileName) {
this.logger.debug(
`[${event}] [end] bookId=${bookId} durationMs=${Date.now() - startedAt} repaired=false coverFound=false - thumbnail repair skipped`,
);
return null;
}
const cover = await readFile(join(dir, coverFileName));
const thumbnail = await generateThumbnail(cover);
await this.publishRepairedThumbnail(dir, thumbnailPath, thumbnail);
this.logger.log(
`[${event}] [end] bookId=${bookId} durationMs=${Date.now() - startedAt} repaired=true source="${sanitizeLogValue(coverFileName)}" - thumbnail repair completed`,
);
return thumbnailPath;
} catch (error) {
const code = (error as NodeJS.ErrnoException | undefined)?.code;
if (code === 'ENOENT' || code === 'ENOTDIR') {
this.logger.debug(
`[${event}] [end] bookId=${bookId} durationMs=${Date.now() - startedAt} repaired=false coverFound=false - thumbnail repair skipped`,
);
return null;
}
const errorClass = error instanceof Error ? error.name : 'Error';
const errorMessage = sanitizeLogValue(error instanceof Error ? error.message : String(error));
this.logger.warn(
`[${event}] [fail] bookId=${bookId} durationMs=${Date.now() - startedAt} errorClass=${errorClass} error="${errorMessage}" - thumbnail repair failed`,
);
return null;
}
}
/**
* Publishes the repaired thumbnail from a fully written temporary file, so an interrupted write
* can never leave a truncated thumbnail at the served path. Nothing validates a thumbnail beyond
* its filename, so a truncated one would be permanent.
*
* A hard link is preferred because it also fails when the path already exists, leaving a cover
* write that landed first in possession of its own thumbnail. Network mounts without hard links
* fall back to rename, which is still atomic and so still cannot publish a partial file; it only
* loses the exclusive create, and losing that race costs a thumbnail that is stale by
* milliseconds rather than one that is broken forever.
*/
private async publishRepairedThumbnail(dir: string, thumbnailPath: string, bytes: Buffer): Promise<void> {
const tempPath = join(dir, `.thumbnail-repair-${randomUUID()}.tmp`);
try {
await writeFile(tempPath, bytes);
try {
await link(tempPath, thumbnailPath);
return;
} catch (error) {
if ((error as NodeJS.ErrnoException | undefined)?.code === 'EEXIST') return;
}
try {
await access(thumbnailPath);
return;
} catch {
// No thumbnail published yet, so this repair still has something to contribute.
}
await rename(tempPath, thumbnailPath);
} finally {
await rm(tempPath, { force: true });
}
}
private async withThumbnailRepairSlot<T>(operation: () => Promise<T>): Promise<T> {
await this.acquireThumbnailRepairSlot();
try {
return await operation();
} finally {
this.releaseThumbnailRepairSlot();
}
}
private async acquireThumbnailRepairSlot(): Promise<void> {
if (this.activeThumbnailRepairs < THUMBNAIL_REPAIR_CONCURRENCY) {
this.activeThumbnailRepairs++;
return;
}
await new Promise<void>((resolve) => this.thumbnailRepairWaiters.push(resolve));
}
private releaseThumbnailRepairSlot(): void {
const next = this.thumbnailRepairWaiters.shift();
if (next) {
next();
return;
}
this.activeThumbnailRepairs--;
}
private normalizeUniqueRelationNames(values: string[]): string[] {
const names = values.map((value) => {
const truncated = normalizeMetadataText(value)?.substring(0, MAX_RELATION_NAME_LENGTH);
@@ -55,12 +55,16 @@ function makeController() {
const bookService = {
resolveDownloadFilename: vi.fn().mockResolvedValue('BadTitle - Author.epub'),
} as never;
const metadataService = {
ensureThumbnailForBook: vi.fn().mockResolvedValue(null),
};
return {
controller: new OpdsController(opdsService, opdsBookService, config, bookService),
controller: new OpdsController(opdsService, opdsBookService, config, bookService, metadataService as never),
opdsService,
opdsBookService,
bookService,
metadataService,
};
}
@@ -319,11 +323,35 @@ describe('OpdsController', () => {
expect(mockCreateReadStream).not.toHaveBeenCalled();
});
it('throws NotFoundException when thumbnail file is missing', async () => {
const { controller } = makeController();
mockStat.mockRejectedValue(new Error('missing thumbnail'));
it('serves a thumbnail rebuilt on demand when the file is missing beside an intact cover', async () => {
const { controller, metadataService } = makeController();
const reply = makeReply();
const stream = { kind: 'repaired-thumbnail-stream' };
mockStat.mockRejectedValueOnce(Object.assign(new Error('missing thumbnail'), { code: 'ENOENT' })).mockResolvedValueOnce({ mtimeMs: 7777 });
metadataService.ensureThumbnailForBook.mockResolvedValueOnce('/books/covers/12/thumbnail.jpg');
mockCreateReadStream.mockReturnValue(stream);
await controller.thumbnail(12, { userId: 1, isSuperuser: false } as never, reply);
expect(metadataService.ensureThumbnailForBook).toHaveBeenCalledWith(12);
expect(reply.header).toHaveBeenCalledWith('ETag', '"7777"');
expect(reply.send).toHaveBeenCalledWith(stream);
});
it('does not attempt a repair when the thumbnail stat fails for a reason other than a missing file', async () => {
const { controller, metadataService } = makeController();
mockStat.mockRejectedValue(Object.assign(new Error('permission denied'), { code: 'EACCES' }));
await expect(controller.thumbnail(42, { userId: 7, isSuperuser: false } as never, makeReply())).rejects.toThrow(NotFoundException);
expect(metadataService.ensureThumbnailForBook).not.toHaveBeenCalled();
});
it('throws NotFoundException when thumbnail file is missing and no cover can rebuild it', async () => {
const { controller, metadataService } = makeController();
mockStat.mockRejectedValue(Object.assign(new Error('missing thumbnail'), { code: 'ENOENT' }));
await expect(controller.thumbnail(42, { userId: 7, isSuperuser: false } as never, makeReply())).rejects.toThrow(NotFoundException);
expect(metadataService.ensureThumbnailForBook).toHaveBeenCalledWith(42);
});
it('downloads file with sanitized attachment name', async () => {
+27 -11
View File
@@ -30,6 +30,7 @@ import { OpdsUser } from './opds-user.decorator';
import { OpdsBookService } from './opds-book.service';
import { OpdsService } from './opds.service';
import { BookService } from '../book/book.service';
import { MetadataService } from '../metadata/metadata.service';
@Controller('opds')
@Public()
@@ -42,6 +43,7 @@ export class OpdsController {
private readonly opdsBookService: OpdsBookService,
private readonly config: ConfigService,
private readonly bookService: BookService,
private readonly metadataService: MetadataService,
) {
this.appDataPath = this.config.get<string>('storage.appDataPath')!;
}
@@ -246,20 +248,34 @@ export class OpdsController {
await this.opdsBookService.validateBookAccess(bookId, user.userId, user.isSuperuser, user.contentFilters);
reply.header('Cross-Origin-Resource-Policy', 'cross-origin');
const thumbnailPath = bookThumbnailPath(this.appDataPath, bookId);
let mtimeMs: number;
try {
const { mtimeMs } = await stat(thumbnailPath);
const etag = `"${Math.floor(mtimeMs)}"`;
if (ifNoneMatch === etag) {
reply.status(304).send();
return;
({ mtimeMs } = await stat(thumbnailPath));
} catch (error) {
// Only a missing thumbnail is repairable. Every other failure keeps the answer this route has
// always given, without spending a cover read and a resize to arrive at it.
const code = (error as NodeJS.ErrnoException | undefined)?.code;
if (code !== 'ENOENT' && code !== 'ENOTDIR') throw new NotFoundException('No thumbnail');
// A thumbnail can be missing beside an intact cover (issue #1475); rebuild it once on demand.
const repaired = await this.metadataService.ensureThumbnailForBook(bookId);
if (!repaired) throw new NotFoundException('No thumbnail');
try {
({ mtimeMs } = await stat(repaired));
} catch {
throw new NotFoundException('No thumbnail');
}
reply.header('Cache-Control', 'no-cache');
reply.header('ETag', etag);
reply.type('image/jpeg');
reply.send(createReadStream(thumbnailPath));
} catch {
throw new NotFoundException('No thumbnail');
}
const etag = `"${Math.floor(mtimeMs)}"`;
if (ifNoneMatch === etag) {
reply.status(304).send();
return;
}
reply.header('Cache-Control', 'no-cache');
reply.header('ETag', etag);
reply.type('image/jpeg');
reply.send(createReadStream(thumbnailPath));
}
@Get(':bookId/download')
+8 -1
View File
@@ -5,6 +5,7 @@ import { MODULE_METADATA } from '@nestjs/common/constants';
import { CommonModule } from '../../common/common.module';
import { AppSettingsModule } from '../app-settings/app-settings.module';
import { BookModule } from '../book/book.module';
import { MetadataModule } from '../metadata/metadata.module';
import { UserModule } from '../user/user.module';
import { OpdsAuthGuard } from './opds-auth.guard';
import { OpdsBookService } from './opds-book.service';
@@ -17,7 +18,13 @@ import { OpdsUserService } from './opds-user.service';
describe('OpdsModule', () => {
it('registers expected module wiring', () => {
expect(Reflect.getMetadata(MODULE_METADATA.IMPORTS, OpdsModule)).toEqual([AppSettingsModule, BookModule, UserModule, CommonModule]);
expect(Reflect.getMetadata(MODULE_METADATA.IMPORTS, OpdsModule)).toEqual([
AppSettingsModule,
BookModule,
MetadataModule,
UserModule,
CommonModule,
]);
expect(Reflect.getMetadata(MODULE_METADATA.CONTROLLERS, OpdsModule)).toEqual([OpdsController, OpdsUserController]);
expect(Reflect.getMetadata(MODULE_METADATA.PROVIDERS, OpdsModule)).toEqual([
OpdsService,
+2 -1
View File
@@ -2,6 +2,7 @@ import { Module } from '@nestjs/common';
import { AppSettingsModule } from '../app-settings/app-settings.module';
import { BookModule } from '../book/book.module';
import { MetadataModule } from '../metadata/metadata.module';
import { UserModule } from '../user/user.module';
import { CommonModule } from '../../common/common.module';
import { OpdsAuthGuard } from './opds-auth.guard';
@@ -13,7 +14,7 @@ import { OpdsUserController } from './opds-user.controller';
import { OpdsUserService } from './opds-user.service';
@Module({
imports: [AppSettingsModule, BookModule, UserModule, CommonModule],
imports: [AppSettingsModule, BookModule, MetadataModule, UserModule, CommonModule],
controllers: [OpdsController, OpdsUserController],
providers: [OpdsService, OpdsBookService, OpdsUserService, OpdsAuthGuard, OpdsEnabledGuard],
exports: [OpdsBookService],