From a5c4dfac16649b4dbf26a3105bb682c6655e7178 Mon Sep 17 00:00:00 2001 From: neon <18559658+neonsolstice@users.noreply.github.com> Date: Tue, 20 Oct 2026 04:17:53 +0000 Subject: [PATCH] fix(series): stop counting unreleased and extra books as missing Hardcover's books_count included novellas, split editions, box sets and announced books, so series named books as missing that are not out yet. Count only released main books, clear the inflated Hardcover totals once on boot, and stop a series row saved without a total from wiping that series' total. Fixes #1861 --- .../constants/app-settings.constants.ts | 1 + server/src/common/series-identity.module.ts | 3 +- ...eries-expected-count-reset.service.test.ts | 102 ++++++++++++++++++ .../series-expected-count-reset.service.ts | 63 +++++++++++ .../series-expected-count.service.test.ts | 48 ++++++++- .../services/series-expected-count.service.ts | 14 ++- server/src/modules/book/book.service.test.ts | 48 ++++++++- server/src/modules/book/book.service.ts | 4 +- .../hardcover/hardcover.client.test.ts | 31 ++++++ .../providers/hardcover/hardcover.client.ts | 34 +++++- .../hardcover/hardcover.mapper.test.ts | 51 ++++++--- .../providers/hardcover/hardcover.mapper.ts | 16 ++- .../providers/hardcover/hardcover.provider.ts | 2 +- .../providers/hardcover/hardcover.types.ts | 3 +- .../provider-cache-declarations.test.ts | 6 +- 15 files changed, 396 insertions(+), 30 deletions(-) create mode 100644 server/src/common/services/series-expected-count-reset.service.test.ts create mode 100644 server/src/common/services/series-expected-count-reset.service.ts diff --git a/server/src/common/constants/app-settings.constants.ts b/server/src/common/constants/app-settings.constants.ts index 73b4936b5..9dbbf8987 100644 --- a/server/src/common/constants/app-settings.constants.ts +++ b/server/src/common/constants/app-settings.constants.ts @@ -25,6 +25,7 @@ export const APP_SETTING_KEYS = { METADATA_SCORE_WEIGHTS: 'metadata_score_weights', AUDIT_RETENTION_DAYS: 'audit_retention_days', AUDIT_TARGET_LABELS_BACKFILLED: 'audit_target_labels_backfilled', + SERIES_HARDCOVER_TOTALS_RESET: 'series_hardcover_totals_reset_v1', INITIAL_SETUP_COMPLETED_AT: 'initial_setup_completed_at', UPDATE_CHECK_ENABLED: 'update_check_enabled', MAX_UPLOAD_SIZE_MB: 'max_upload_size_mb', diff --git a/server/src/common/series-identity.module.ts b/server/src/common/series-identity.module.ts index 556a92639..564741d9b 100644 --- a/server/src/common/series-identity.module.ts +++ b/server/src/common/series-identity.module.ts @@ -1,6 +1,7 @@ import { Global, Module } from '@nestjs/common'; import { DbModule } from '../db/db.module'; +import { SeriesExpectedCountResetService } from './services/series-expected-count-reset.service'; import { SeriesExpectedCountService } from './services/series-expected-count.service'; import { SeriesIdentityService } from './services/series-identity.service'; import { SeriesMembershipService } from './services/series-membership.service'; @@ -8,7 +9,7 @@ import { SeriesMembershipService } from './services/series-membership.service'; @Global() @Module({ imports: [DbModule], - providers: [SeriesIdentityService, SeriesMembershipService, SeriesExpectedCountService], + providers: [SeriesIdentityService, SeriesMembershipService, SeriesExpectedCountService, SeriesExpectedCountResetService], exports: [SeriesIdentityService, SeriesMembershipService, SeriesExpectedCountService], }) export class SeriesIdentityModule {} diff --git a/server/src/common/services/series-expected-count-reset.service.test.ts b/server/src/common/services/series-expected-count-reset.service.test.ts new file mode 100644 index 000000000..8cf57a5c2 --- /dev/null +++ b/server/src/common/services/series-expected-count-reset.service.test.ts @@ -0,0 +1,102 @@ +import { MetadataProviderKey } from '@bookorbit/types'; + +import { APP_SETTING_KEYS } from '../constants/app-settings.constants'; +import { SeriesExpectedCountResetService } from './series-expected-count-reset.service'; + +/** Collects the primitives drizzle binds into a query, so tests can assert on parameters rather than SQL text. */ +function boundValues(node: unknown, out: unknown[] = [], seen = new Set()): unknown[] { + if (node === null || node === undefined || seen.has(node)) return out; + if (typeof node === 'string' || typeof node === 'number') { + out.push(node); + return out; + } + if (typeof node !== 'object') return out; + seen.add(node); + for (const value of Object.values(node as Record)) boundValues(value, out, seen); + return out; +} + +function makeDb(options: { markerWon: boolean; clearedIds?: number[]; updateError?: Error }) { + const insertReturning = vi.fn().mockResolvedValue(options.markerWon ? [{ key: APP_SETTING_KEYS.SERIES_HARDCOVER_TOTALS_RESET }] : []); + const onConflictDoNothing = vi.fn().mockReturnValue({ returning: insertReturning }); + const values = vi.fn().mockReturnValue({ onConflictDoNothing }); + const insert = vi.fn().mockReturnValue({ values }); + + const updateReturning = options.updateError + ? vi.fn().mockRejectedValue(options.updateError) + : vi.fn().mockResolvedValue((options.clearedIds ?? []).map((id) => ({ id }))); + const where = vi.fn().mockReturnValue({ returning: updateReturning }); + const set = vi.fn().mockReturnValue({ where }); + const update = vi.fn().mockReturnValue({ set }); + + const tx = { insert, update }; + const transaction = vi.fn((callback: (value: typeof tx) => unknown) => callback(tx)); + return { db: { transaction } as never, transaction, values, onConflictDoNothing, update, set, where }; +} + +function makeService(db: never) { + const service = new SeriesExpectedCountResetService(db); + const logger = (service as unknown as { logger: { log: (m: string) => void; warn: (m: string) => void } }).logger; + return { + service, + log: vi.spyOn(logger, 'log').mockImplementation(() => undefined), + warn: vi.spyOn(logger, 'warn').mockImplementation(() => undefined), + }; +} + +describe('SeriesExpectedCountResetService', () => { + it('clears only totals recorded from Hardcover, and claims the marker in the same transaction', async () => { + const { db, transaction, values, onConflictDoNothing, set, where } = makeDb({ markerWon: true, clearedIds: [4, 9, 12] }); + const { service } = makeService(db); + + await expect(service.clearHardcoverTotalsOnce()).resolves.toBe(3); + + expect(transaction).toHaveBeenCalledTimes(1); + expect(values).toHaveBeenCalledWith(expect.objectContaining({ key: APP_SETTING_KEYS.SERIES_HARDCOVER_TOTALS_RESET })); + expect(onConflictDoNothing).toHaveBeenCalledTimes(1); + expect(set).toHaveBeenCalledWith( + expect.objectContaining({ expectedBookCount: null, expectedBookCountSource: null, expectedBookCountUpdatedAt: expect.any(Date) }), + ); + const bound = boundValues(where.mock.calls[0]?.[0]); + expect(bound).toContain(MetadataProviderKey.HARDCOVER); + expect(bound).not.toContain('manual'); + expect(bound).not.toContain('embedded'); + }); + + it('touches nothing once an earlier boot has claimed the marker', async () => { + const { db, update } = makeDb({ markerWon: false }); + const { service } = makeService(db); + + await expect(service.clearHardcoverTotalsOnce()).resolves.toBeNull(); + expect(update).not.toHaveBeenCalled(); + }); + + it('logs the count when it runs at startup', async () => { + const { db } = makeDb({ markerWon: true, clearedIds: [1, 2] }); + const { service, log } = makeService(db); + + await service.onApplicationBootstrap(); + + expect(log).toHaveBeenCalledWith(expect.stringMatching(/^\[series\.hardcover_total_reset\] \[end\] durationMs=\d+ cleared=2 - /)); + }); + + it('stays quiet at startup once the reset has already run', async () => { + const { db } = makeDb({ markerWon: false }); + const { service, log, warn } = makeService(db); + + await service.onApplicationBootstrap(); + + expect(log).not.toHaveBeenCalled(); + expect(warn).not.toHaveBeenCalled(); + }); + + it('logs a failure without blocking startup', async () => { + const { db } = makeDb({ markerWon: true, updateError: new Error('statement timeout') }); + const { service, warn } = makeService(db); + + await expect(service.onApplicationBootstrap()).resolves.toBeUndefined(); + + expect(warn).toHaveBeenCalledWith(expect.stringContaining('[series.hardcover_total_reset] [fail]')); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('error="statement timeout"')); + }); +}); diff --git a/server/src/common/services/series-expected-count-reset.service.ts b/server/src/common/services/series-expected-count-reset.service.ts new file mode 100644 index 000000000..0c6a6fb14 --- /dev/null +++ b/server/src/common/services/series-expected-count-reset.service.ts @@ -0,0 +1,63 @@ +import { Inject, Injectable, Logger, OnApplicationBootstrap } from '@nestjs/common'; +import { MetadataProviderKey } from '@bookorbit/types'; +import { eq } from 'drizzle-orm'; +import type { NodePgDatabase } from 'drizzle-orm/node-postgres'; + +import { DB } from '../../db'; +import * as schema from '../../db/schema'; +import { appSettings, bookSeries } from '../../db/schema'; +import { APP_SETTING_KEYS } from '../constants/app-settings.constants'; +import { sanitizeLogValue } from '../utils/log-sanitize.utils'; + +type Db = NodePgDatabase; + +/** + * Clears every series total recorded from Hardcover before it counted only released main books. + * Its old count included novellas, split editions, box sets and announced books, so series showed + * books as missing that do not exist yet. Stored totals only rise, so a corrected answer could + * never replace an inflated one. The next metadata fetch records the corrected total. + * + * Runs once per database: the marker is written in the same transaction as the reset. A failure is + * logged and swallowed, and the next boot tries again. + */ +@Injectable() +export class SeriesExpectedCountResetService implements OnApplicationBootstrap { + private readonly logger = new Logger(SeriesExpectedCountResetService.name); + + constructor(@Inject(DB) private readonly db: Db) {} + + async onApplicationBootstrap(): Promise { + const event = 'series.hardcover_total_reset'; + const startedAt = Date.now(); + try { + const cleared = await this.clearHardcoverTotalsOnce(); + if (cleared === null) return; + this.logger.log(`[${event}] [end] durationMs=${Date.now() - startedAt} cleared=${cleared} - Hardcover series totals cleared`); + } catch (err) { + const errorClass = err instanceof Error ? err.name : 'Error'; + const errorMessage = sanitizeLogValue(err instanceof Error ? err.message : String(err)); + this.logger.warn( + `[${event}] [fail] durationMs=${Date.now() - startedAt} errorClass=${errorClass} error="${errorMessage}" - Hardcover series total reset failed`, + ); + } + } + + /** Returns how many totals were cleared, or null when an earlier boot already did it. */ + async clearHardcoverTotalsOnce(): Promise { + return this.db.transaction(async (tx) => { + const [marker] = await tx + .insert(appSettings) + .values({ key: APP_SETTING_KEYS.SERIES_HARDCOVER_TOTALS_RESET, value: new Date().toISOString() }) + .onConflictDoNothing({ target: appSettings.key }) + .returning({ key: appSettings.key }); + if (!marker) return null; + + const rows = await tx + .update(bookSeries) + .set({ expectedBookCount: null, expectedBookCountSource: null, expectedBookCountUpdatedAt: new Date() }) + .where(eq(bookSeries.expectedBookCountSource, MetadataProviderKey.HARDCOVER)) + .returning({ id: bookSeries.id }); + return rows.length; + }); + } +} diff --git a/server/src/common/services/series-expected-count.service.test.ts b/server/src/common/services/series-expected-count.service.test.ts index 49f93d6b7..1b4c02ec6 100644 --- a/server/src/common/services/series-expected-count.service.test.ts +++ b/server/src/common/services/series-expected-count.service.test.ts @@ -284,11 +284,55 @@ describe('SeriesExpectedCountService', () => { await service.setManual('Dune', 2); expect(set).toHaveBeenCalledWith(expect.objectContaining({ expectedBookCount: 2 })); - // No comparison on the stored total: a person stating the length outright must win + // No ordering on the stored total: a person stating the length outright must win // in both directions, which is the escape hatch for a wrong automated maximum. const operators = sqlOperators(where.mock.calls[0]?.[0]); expect(operators).not.toContain(' < '); - expect(operators).not.toContain(' <> '); + expect(operators).not.toContain(' > '); + }); + + it('leaves an unchanged total alone, so re-saving a provider total keeps its source', async () => { + const identity = makeSeriesIdentity({ dune: 101 }); + const { db, where } = makeDb(); + const service = new SeriesExpectedCountService(db, identity as never); + + await service.setManual('Dune', 6); + await service.setManual('Dune', null); + + const setOperators = sqlOperators(where.mock.calls[0]?.[0]); + expect(setOperators).toContain(' <> '); + expect(setOperators).toContain(' is null'); + expect(sqlOperators(where.mock.calls[1]?.[0])).toContain(' is not null'); + }); + + it('ignores an empty total on a series the book was not already in, since no total was shown for it', async () => { + const identity = makeSeriesIdentity({ dune: 101, 'children of dune': 102 }); + const { db, update } = makeDb(); + const service = new SeriesExpectedCountService(db, identity as never); + + await expect(service.setManual('Children of Dune', null, undefined, { previousSeriesIds: new Set([101]) })).resolves.toBe(true); + + expect(update).not.toHaveBeenCalled(); + }); + + it('still clears a total the editor showed for a series the book was already in', async () => { + const identity = makeSeriesIdentity({ dune: 101 }); + const { db, set } = makeDb(); + const service = new SeriesExpectedCountService(db, identity as never); + + await service.setManual('Dune', null, undefined, { previousSeriesIds: new Set([101]) }); + + expect(set).toHaveBeenCalledWith(expect.objectContaining({ expectedBookCount: null, expectedBookCountSource: null })); + }); + + it('applies a typed total on a newly added series', async () => { + const identity = makeSeriesIdentity({ dune: 101, 'children of dune': 102 }); + const { db, set } = makeDb(); + const service = new SeriesExpectedCountService(db, identity as never); + + await service.setManual('Children of Dune', 3, undefined, { previousSeriesIds: new Set([101]) }); + + expect(set).toHaveBeenCalledWith(expect.objectContaining({ expectedBookCount: 3, expectedBookCountSource: MANUAL_SERIES_TOTAL_SOURCE })); }); it('reports false without writing when the series is unknown or unnamed', async () => { diff --git a/server/src/common/services/series-expected-count.service.ts b/server/src/common/services/series-expected-count.service.ts index d415c6572..32b7264a6 100644 --- a/server/src/common/services/series-expected-count.service.ts +++ b/server/src/common/services/series-expected-count.service.ts @@ -1,5 +1,5 @@ import { Inject, Injectable, Logger } from '@nestjs/common'; -import { and, eq, isNull, lt, or } from 'drizzle-orm'; +import { and, eq, isNotNull, isNull, lt, ne, or } from 'drizzle-orm'; import type { NodePgDatabase } from 'drizzle-orm/node-postgres'; import type { MetadataCandidate } from '@bookorbit/types'; @@ -75,11 +75,17 @@ export class SeriesExpectedCountService { * stating the length outright, and failures propagate so a save cannot silently do nothing. * * The row is shared, so this changes the total every user sees for the series. + * + * `previousSeriesIds` are the series the book was in before the save, the only rows whose total + * an editor showed. Both editors send null for a row added during the edit, which means "not + * given", so a null only clears one of these. An unchanged total is left alone, so re-saving a + * provider's total keeps its source. */ async setManual( seriesName: string | null | undefined, expectedBookCount: number | null, executor: ExpectedCountExecutor = this.db, + options: { previousSeriesIds?: ReadonlySet } = {}, ): Promise { const displayName = this.seriesIdentity.normalizeDisplayName(seriesName); if (!displayName) return false; @@ -88,6 +94,10 @@ export class SeriesExpectedCountService { if (seriesId == null) return false; const total = expectedBookCount === null ? null : (normalizeSeriesTotalBooks(expectedBookCount) ?? null); + if (total === null && options.previousSeriesIds && !options.previousSeriesIds.has(seriesId)) return true; + + const changed = + total === null ? isNotNull(bookSeries.expectedBookCount) : or(isNull(bookSeries.expectedBookCount), ne(bookSeries.expectedBookCount, total)); await executor .update(bookSeries) .set({ @@ -95,7 +105,7 @@ export class SeriesExpectedCountService { expectedBookCountSource: total === null ? null : MANUAL_SERIES_TOTAL_SOURCE, expectedBookCountUpdatedAt: new Date(), }) - .where(eq(bookSeries.id, seriesId)); + .where(and(eq(bookSeries.id, seriesId), changed)); return true; } diff --git a/server/src/modules/book/book.service.test.ts b/server/src/modules/book/book.service.test.ts index 0dd6428ae..3424b3cd2 100644 --- a/server/src/modules/book/book.service.test.ts +++ b/server/src/modules/book/book.service.test.ts @@ -121,7 +121,14 @@ function makeMetadataFetchDiagnostics(overrides: Partial { ); }); + it('updateMetadata tells the total writer which series the book was already in before replacing them', async () => { + const calls: string[] = []; + const seriesMemberships = { + findByBookId: vi.fn(() => { + calls.push('find'); + return Promise.resolve([{ seriesId: 31, seriesName: 'Harry Potter', seriesIndex: '1', displayOrder: 0, expectedBookCount: 7 }]); + }), + replaceForBook: vi.fn(() => { + calls.push('replace'); + return Promise.resolve([]); + }), + }; + const seriesExpectedCount = { setManual: vi.fn().mockResolvedValue(true) }; + const { service } = makeService({ seriesMemberships, seriesExpectedCount }); + const user = makeUser(); + vi.spyOn(service, 'verifyBookAccess').mockResolvedValue(undefined); + vi.spyOn(service, 'getDetail').mockResolvedValue({ id: 5 } as never); + + await service.updateMetadata( + 5, + { + seriesMemberships: [ + { seriesName: 'Harry Potter', seriesIndex: '1', expectedBookCount: 7 }, + { seriesName: 'Hogwarts Library', seriesIndex: null, expectedBookCount: null }, + ], + }, + user, + ); + + expect(calls).toEqual(['find', 'replace']); + const options = { previousSeriesIds: new Set([31]) }; + expect(seriesExpectedCount.setManual).toHaveBeenNthCalledWith(1, 'Harry Potter', 7, expect.anything(), options); + expect(seriesExpectedCount.setManual).toHaveBeenNthCalledWith(2, 'Hogwarts Library', null, expect.anything(), options); + }); + it('updateMetadata with empty communityRatings clears all existing ratings', async () => { const { service, bookRepo } = makeService(); const user = makeUser(); diff --git a/server/src/modules/book/book.service.ts b/server/src/modules/book/book.service.ts index cc9417b4e..98352f282 100644 --- a/server/src/modules/book/book.service.ts +++ b/server/src/modules/book/book.service.ts @@ -1965,11 +1965,13 @@ export class BookService { } if (dto.seriesMemberships !== undefined) { + const previous = (await this.seriesMemberships?.findByBookId(id, tx)) ?? []; + const totalOptions = { previousSeriesIds: new Set(previous.map((membership) => membership.seriesId)) }; await this.seriesMemberships?.replaceForBook(id, dto.seriesMemberships, tx); // After replaceForBook, because it is what creates any series row the editor named. for (const membership of dto.seriesMemberships ?? []) { if (membership.expectedBookCount === undefined) continue; - await this.seriesExpectedCount?.setManual(membership.seriesName, membership.expectedBookCount ?? null, tx); + await this.seriesExpectedCount?.setManual(membership.seriesName, membership.expectedBookCount ?? null, tx, totalOptions); } } this.throwIfMetadataUpdateFailpoint('afterSeriesMembershipsReplace'); diff --git a/server/src/modules/metadata-fetch/providers/hardcover/hardcover.client.test.ts b/server/src/modules/metadata-fetch/providers/hardcover/hardcover.client.test.ts index 5cb85ee78..8d25b4316 100644 --- a/server/src/modules/metadata-fetch/providers/hardcover/hardcover.client.test.ts +++ b/server/src/modules/metadata-fetch/providers/hardcover/hardcover.client.test.ts @@ -93,6 +93,37 @@ describe('HardcoverClient', () => { expect(body.query).toMatch(/\bcached_tags\b/); }); + it.each([ + ['ISBN search', () => client.searchByIsbn('9780756404079', apiKey)], + ['stored ID lookup', () => client.lookupBySlug('the-name-of-the-wind', apiKey)], + ])('asks for the last released main book instead of every series entry for %s', async (_label, request) => { + vi.useFakeTimers({ toFake: ['Date'] }); + vi.setSystemTime(new Date('2026-10-19T23:30:00Z')); + try { + const mockFetch = vi.mocked(fetchWithThrottleModule.fetchWithThrottle); + mockFetch.mockResolvedValue({ + ok: true, + status: 200, + json: () => Promise.resolve({ data: { books: [] } }), + } as Response); + + await request(); + + const options = mockFetch.mock.calls[0][1]; + if (typeof options?.body !== 'string') throw new TypeError('Expected a JSON request body'); + const body = JSON.parse(options.body) as { query: string; variables: Record }; + expect(body.query).not.toMatch(/\bbooks_count\b/); + expect(body.query).toMatch(/\bprimary_books_count\b/); + expect(body.query).toMatch(/release_date:\s*\{\s*_lte:\s*\$releasedBy\s*\}/); + expect(body.query).toMatch(/canonical_id:\s*\{\s*_is_null:\s*true\s*\}/); + expect(body.query).toMatch(/compilation:\s*\{\s*_eq:\s*false\s*\}/); + expect(body.query).toMatch(/\$releasedBy:\s*date!/); + expect(body.variables.releasedBy).toBe('2026-10-19'); + } finally { + vi.useRealTimers(); + } + }); + it('returns empty array when API returns non-ok status', async () => { const mockFetch = vi.mocked(fetchWithThrottleModule.fetchWithThrottle); mockFetch.mockResolvedValue({ diff --git a/server/src/modules/metadata-fetch/providers/hardcover/hardcover.client.ts b/server/src/modules/metadata-fetch/providers/hardcover/hardcover.client.ts index c5880942b..2d87f8eca 100644 --- a/server/src/modules/metadata-fetch/providers/hardcover/hardcover.client.ts +++ b/server/src/modules/metadata-fetch/providers/hardcover/hardcover.client.ts @@ -10,6 +10,24 @@ import { HardcoverBookWithEditions, HardcoverBooksResponse, HardcoverSearchDocum const GRAPHQL_ENDPOINT = 'https://api.hardcover.app/v1/graphql'; +// The series total feeds the series gap finder, which reads it as the last book number, so it +// must be a released main-series number. books_count also counts novellas, split editions, box +// sets and announced books. Translations carry their own odd numbering, so only canonical books +// are considered. +const SERIES_FIELDS = ` + name + primary_books_count + latest_released: book_series( + where: { + position: { _is_null: false } + compilation: { _eq: false } + book: { canonical_id: { _is_null: true }, release_date: { _lte: $releasedBy } } + } + order_by: { position: desc } + limit: 1 + ) { position } +`; + const BOOK_FIELDS = ` id slug @@ -18,7 +36,7 @@ const BOOK_FIELDS = ` description cached_contributors cached_tags - featured_book_series { series { name books_count } position } + featured_book_series { series { ${SERIES_FIELDS} } position } rating ratings_count pages @@ -49,7 +67,7 @@ const EDITION_FIELDS = ` const EDITION_LOOKUP_LIMIT = 50; const SEARCH_BY_ISBN_QUERY = ` - query BookSearchByIsbn($isbn: String!) { + query BookSearchByIsbn($isbn: String!, $releasedBy: date!) { books(where: { editions: { _or: [{ isbn_13: { _eq: $isbn } }, { isbn_10: { _eq: $isbn } }] } }) { ${BOOK_FIELDS} editions(where: { _or: [{ isbn_13: { _eq: $isbn } }, { isbn_10: { _eq: $isbn } }] }) { @@ -68,7 +86,7 @@ const SEARCH_BOOKS_QUERY = ` `; const LOOKUP_BY_SLUG_QUERY = ` - query BookBySlug($slug: String!) { + query BookBySlug($slug: String!, $releasedBy: date!) { books(where: { slug: { _eq: $slug } }) { ${BOOK_FIELDS} editions(limit: ${EDITION_LOOKUP_LIMIT}) { @@ -92,6 +110,12 @@ class RateLimiter { } } +// Deliberately not part of the provider cache key, which would expire every answer at midnight. A +// cached answer from before a release can only undercount, and stored series totals only rise. +function releasedBy(): string { + return new Date().toISOString().slice(0, 10); +} + // GraphQL reports query and permission faults in a 200 body, sometimes alongside partial data. function reportGraphqlBodyQuality(body: unknown, op: string): void { const record = typeof body === 'object' && body !== null ? (body as { errors?: unknown; data?: unknown }) : null; @@ -106,7 +130,7 @@ export class HardcoverClient { private readonly rateLimiter = new RateLimiter(); async searchByIsbn(isbn: string, apiKey: string, signal?: AbortSignal): Promise { - const body = await this.post('search-by-isbn', SEARCH_BY_ISBN_QUERY, { isbn }, apiKey, signal); + const body = await this.post('search-by-isbn', SEARCH_BY_ISBN_QUERY, { isbn, releasedBy: releasedBy() }, apiKey, signal); if (body && !Array.isArray(body.data?.books)) markProviderFailed('hardcover.search-by-isbn: response without books'); return body?.data?.books ?? []; } @@ -127,7 +151,7 @@ export class HardcoverClient { } async lookupBySlug(slug: string, apiKey: string, signal?: AbortSignal): Promise { - const body = await this.post('lookup', LOOKUP_BY_SLUG_QUERY, { slug }, apiKey, signal); + const body = await this.post('lookup', LOOKUP_BY_SLUG_QUERY, { slug, releasedBy: releasedBy() }, apiKey, signal); if (body && !Array.isArray(body.data?.books)) markProviderFailed('hardcover.lookup: response without books'); return body?.data?.books?.[0] ?? null; } diff --git a/server/src/modules/metadata-fetch/providers/hardcover/hardcover.mapper.test.ts b/server/src/modules/metadata-fetch/providers/hardcover/hardcover.mapper.test.ts index 41dd5df72..1115e1053 100644 --- a/server/src/modules/metadata-fetch/providers/hardcover/hardcover.mapper.test.ts +++ b/server/src/modules/metadata-fetch/providers/hardcover/hardcover.mapper.test.ts @@ -35,7 +35,10 @@ const baseBook: HardcoverBookWithEditions = { { tag: 'Fiction', count: 8 }, ], }, - featured_book_series: { series: { name: 'The Kingkiller Chronicle', books_count: 3 }, position: 1 }, + featured_book_series: { + series: { name: 'The Kingkiller Chronicle', primary_books_count: 3, latest_released: [{ position: 2 }] }, + position: 1, + }, rating: 4.42, ratings_count: 12345, pages: 662, @@ -299,7 +302,7 @@ describe('mapBookWithEditions', () => { genres: ['Fantasy', 'Fiction'], seriesName: 'The Kingkiller Chronicle', seriesIndex: '1', - seriesTotalBooks: 3, + seriesTotalBooks: 2, communityRating: 4.42, communityRatingCount: 12345, coverUrl: 'https://assets.hardcover.app/edition-cover.jpg', @@ -314,22 +317,45 @@ describe('mapBookWithEditions', () => { }); describe('series total books', () => { - function withBooksCount(booksCount: unknown): HardcoverBookWithEditions { + function withSeries(series: { primary_books_count?: unknown; latest_released?: unknown }): HardcoverBookWithEditions { return { ...baseBook, featured_book_series: { - series: { name: 'The Kingkiller Chronicle', books_count: booksCount as number | undefined }, + series: { name: 'Red Rising Saga', ...series } as NonNullable['series'], position: 1, }, }; } - it('maps the series book count Hardcover already returns', () => { - expect(mapBookWithEditions(withBooksCount(7))[0].seriesTotalBooks).toBe(7); + function totalFor(series: { primary_books_count?: unknown; latest_released?: unknown }): number | undefined { + return mapBookWithEditions(withSeries(series))[0].seriesTotalBooks; + } + + it('is the number of the last released main book, not every entry Hardcover lists', () => { + expect(totalFor({ primary_books_count: 7, latest_released: [{ position: 6 }] })).toBe(6); }); - it('is undefined when Hardcover omits the count', () => { - expect(mapBookWithEditions(withBooksCount(undefined))[0].seriesTotalBooks).toBeUndefined(); + it('names the book a released split part or novella follows', () => { + expect(totalFor({ primary_books_count: 10, latest_released: [{ position: 5.2 }] })).toBe(5); + }); + + it('caps a stray high number at the count of main books', () => { + expect(totalFor({ primary_books_count: 14, latest_released: [{ position: 36 }] })).toBe(14); + }); + + it('uses the released number alone when Hardcover has no main book count', () => { + expect(totalFor({ primary_books_count: null, latest_released: [{ position: 4 }] })).toBe(4); + expect(totalFor({ primary_books_count: 0, latest_released: [{ position: 4 }] })).toBe(4); + }); + + it('is undefined when no numbered book has been released', () => { + expect(totalFor({ primary_books_count: 3, latest_released: [] })).toBeUndefined(); + expect(totalFor({ primary_books_count: 3 })).toBeUndefined(); + expect(totalFor({ primary_books_count: 3, latest_released: [{ position: null }] })).toBeUndefined(); + }); + + it('is undefined when only a prequel numbered below one has been released', () => { + expect(totalFor({ primary_books_count: 3, latest_released: [{ position: 0.5 }] })).toBeUndefined(); }); it('is undefined when the series itself is absent', () => { @@ -337,10 +363,11 @@ describe('mapBookWithEditions', () => { expect(mapBookWithEditions(book)[0].seriesTotalBooks).toBeUndefined(); }); - it('rejects an out-of-range count rather than trusting the payload', () => { - expect(mapBookWithEditions(withBooksCount(0))[0].seriesTotalBooks).toBeUndefined(); - expect(mapBookWithEditions(withBooksCount(-4))[0].seriesTotalBooks).toBeUndefined(); - expect(mapBookWithEditions(withBooksCount(10_001))[0].seriesTotalBooks).toBeUndefined(); + it('rejects an unusable number rather than trusting the payload', () => { + expect(totalFor({ latest_released: [{ position: -4 }] })).toBeUndefined(); + expect(totalFor({ latest_released: [{ position: 10_001 }] })).toBeUndefined(); + expect(totalFor({ latest_released: [{ position: '6' }] })).toBeUndefined(); + expect(totalFor({ latest_released: [{ position: Number.NaN }] })).toBeUndefined(); }); }); diff --git a/server/src/modules/metadata-fetch/providers/hardcover/hardcover.mapper.ts b/server/src/modules/metadata-fetch/providers/hardcover/hardcover.mapper.ts index 5d8a31c1c..e845bbb96 100644 --- a/server/src/modules/metadata-fetch/providers/hardcover/hardcover.mapper.ts +++ b/server/src/modules/metadata-fetch/providers/hardcover/hardcover.mapper.ts @@ -22,6 +22,20 @@ function parseDate(releaseYear: number | undefined | null, releaseDate: string | return year === undefined || year === publishedYearFromDateKey(publishedDate) ? publishedDate : undefined; } +type HardcoverSeries = NonNullable['series']>; + +/** + * The number of the last released main book. Split parts and novellas sit at fractions after the + * book they follow, so the integer part names it. primary_books_count caps a stray high number. + */ +function releasedSeriesTotal(series: HardcoverSeries | undefined): number | undefined { + const position = series?.latest_released?.[0]?.position; + if (typeof position !== 'number' || !Number.isFinite(position)) return undefined; + const released = Math.floor(position); + const cap = normalizeSeriesTotalBooks(series?.primary_books_count); + return normalizeSeriesTotalBooks(cap === undefined ? released : Math.min(released, cap)); +} + function extractAuthorsFromContributors(contributors: HardcoverCachedContributor[] | undefined): string[] { if (!contributors) return []; return contributors @@ -197,7 +211,7 @@ function mapEdition(edition: HardcoverEdition, book: HardcoverBookWithEditions): ...(genres ? { genres } : {}), seriesName: book.featured_book_series?.series?.name, seriesIndex: parseSeriesIndex(book.featured_book_series?.position) ?? undefined, - seriesTotalBooks: normalizeSeriesTotalBooks(book.featured_book_series?.series?.books_count), + seriesTotalBooks: releasedSeriesTotal(book.featured_book_series?.series), coverUrl: edition.image?.url ?? book.image?.url, ...editionCover(edition, book), sourceUrl: `https://hardcover.app/books/${book.slug}`, diff --git a/server/src/modules/metadata-fetch/providers/hardcover/hardcover.provider.ts b/server/src/modules/metadata-fetch/providers/hardcover/hardcover.provider.ts index 7794ecfad..dd963e8a3 100644 --- a/server/src/modules/metadata-fetch/providers/hardcover/hardcover.provider.ts +++ b/server/src/modules/metadata-fetch/providers/hardcover/hardcover.provider.ts @@ -44,7 +44,7 @@ export class HardcoverProvider implements IdentifiableProvider { private readonly logger = new Logger(HardcoverProvider.name); readonly cache: ProviderCacheSpec = { - version: 1, + version: 2, trustEmpty: true, context: async () => { const { enabled, apiKey } = (await this.providerConfig.getConfig()).hardcover; diff --git a/server/src/modules/metadata-fetch/providers/hardcover/hardcover.types.ts b/server/src/modules/metadata-fetch/providers/hardcover/hardcover.types.ts index 777313fcc..66f333c93 100644 --- a/server/src/modules/metadata-fetch/providers/hardcover/hardcover.types.ts +++ b/server/src/modules/metadata-fetch/providers/hardcover/hardcover.types.ts @@ -56,7 +56,8 @@ export interface HardcoverBookWithEditions { featured_book_series?: { series?: { name?: string; - books_count?: number; + primary_books_count?: number | null; + latest_released?: { position?: number | null }[]; }; position?: number | null; }; diff --git a/server/src/modules/metadata-fetch/providers/provider-cache-declarations.test.ts b/server/src/modules/metadata-fetch/providers/provider-cache-declarations.test.ts index 113c137d7..a7107560c 100644 --- a/server/src/modules/metadata-fetch/providers/provider-cache-declarations.test.ts +++ b/server/src/modules/metadata-fetch/providers/provider-cache-declarations.test.ts @@ -326,7 +326,11 @@ describe('provider cache declarations', () => { describe.each(CASES)('$key', (testCase) => { it('declares the current mapping version', () => { - expect(specOf(testCase.key).version).toBe(testCase.key === MetadataProviderKey.COMICVINE ? 2 : 1); + const bumped: Partial> = { + [MetadataProviderKey.COMICVINE]: 2, + [MetadataProviderKey.HARDCOVER]: 2, + }; + expect(specOf(testCase.key).version).toBe(bumped[testCase.key] ?? 1); }); it('keys a search on the fields it reads', () => {