diff --git a/docs/API.md b/docs/API.md index b15b815..4c40894 100644 --- a/docs/API.md +++ b/docs/API.md @@ -653,11 +653,12 @@ the server has `GOOGLE_BOOKS_API_KEY`, then Amazon's "Print length" via SearchAP #### PUT /api/v1/documents/{document}/info -Manual fixes when the lookup got it wrong: `{"cover_url": "https://...", "page_count": 433}` (either -field optional). Manual values stick, since lookups only fill blanks; `null` clears a field so it's -looked up again. `language` (a BCP 47 tag) can be set the same way and then stays over device -syncs; `null` clears it so the next device sync fills it again. Returns the stored -`{document, cover_url, page_count, title, author, language}`. +Manual fixes when the lookup got it wrong: `{"cover_url": "https://...", "page_count": 433, +"genres": ["Fantasy", "Heist"]}` (every field optional). Manual values stick, since lookups only fill +blanks and Hardcover's lookup leaves manual genres alone; `null` clears a field so it's looked up +again. `genres` takes up to 10 names of at most 64 characters. `language` (a BCP 47 tag) can be set +the same way and then stays over device syncs; `null` clears it so the next device sync fills it +again. Returns the stored `{document, cover_url, page_count, title, author, language, genres}`. #### GET /api/v1/documents/{document}/cover/candidates?q= diff --git a/migrations/0033_manual_genres.sql b/migrations/0033_manual_genres.sql new file mode 100644 index 0000000..e706fcb --- /dev/null +++ b/migrations/0033_manual_genres.sql @@ -0,0 +1,2 @@ +-- Genres set through PUT /api/v1/documents/:document/info stick over Hardcover lookups. +ALTER TABLE documents ADD COLUMN genres_manual INTEGER NOT NULL DEFAULT 0; diff --git a/src/models/cover.ts b/src/models/cover.ts index 51072b5..9a29bbf 100644 --- a/src/models/cover.ts +++ b/src/models/cover.ts @@ -215,11 +215,11 @@ async function sharedBookInfo( return info; } -/** Store Hardcover's details on a document (a miss just records the check). */ +/** Store Hardcover's details on a document (a miss just records the check). Genres set by hand stay. */ function saveHardcover(db: DB, userId: number, document: string, hc: HardcoverBook | null): void { const json = (v: string[] | undefined) => (v?.length ? JSON.stringify(v) : null); db.prepare( - `UPDATE documents SET hc_id = ?, hc_slug = ?, moods = ?, genres = ?, content_warnings = ?, rating = ?, + `UPDATE documents SET hc_id = ?, hc_slug = ?, moods = ?, genres = CASE WHEN genres_manual THEN genres ELSE ? END, content_warnings = ?, rating = ?, series = ?, hc_series_id = ?, series_position = ?, release_year = ?, description = ?, hc_checked_at = ? WHERE user_id = ? AND document = ?` ).run( diff --git a/src/models/merge.ts b/src/models/merge.ts index 5116e83..048de77 100644 --- a/src/models/merge.ts +++ b/src/models/merge.ts @@ -148,8 +148,8 @@ export function mergeDocuments( // documents: canonical metadata wins, alias fills the gaps. db.prepare( - `INSERT INTO documents (user_id, document, title, author, filename, filesize, status, status_at, pause_reason, start_date, finished_date, cover_url, page_count, language, updated_at) - SELECT user_id, ?, title, author, filename, filesize, status, status_at, pause_reason, start_date, finished_date, cover_url, page_count, language, ? FROM documents WHERE user_id = ? AND document = ? + `INSERT INTO documents (user_id, document, title, author, filename, filesize, status, status_at, pause_reason, start_date, finished_date, cover_url, page_count, language, genres, genres_manual, updated_at) + SELECT user_id, ?, title, author, filename, filesize, status, status_at, pause_reason, start_date, finished_date, cover_url, page_count, language, genres, genres_manual, ? FROM documents WHERE user_id = ? AND document = ? ON CONFLICT(user_id, document) DO UPDATE SET title = COALESCE(documents.title, excluded.title), author = COALESCE(documents.author, excluded.author), @@ -163,6 +163,10 @@ export function mergeDocuments( cover_url = COALESCE(documents.cover_url, excluded.cover_url), page_count = COALESCE(documents.page_count, excluded.page_count), language = COALESCE(documents.language, excluded.language), + -- Genres set by hand beat Hardcover's; otherwise the kept document's win. + genres = CASE WHEN excluded.genres_manual AND NOT documents.genres_manual THEN excluded.genres + ELSE COALESCE(documents.genres, excluded.genres) END, + genres_manual = MAX(documents.genres_manual, excluded.genres_manual), updated_at = excluded.updated_at` ).run(into, now, userId, from); db.prepare('DELETE FROM documents WHERE user_id = ? AND document = ?').run(userId, from); diff --git a/src/routes/v1/documents.ts b/src/routes/v1/documents.ts index 10477f7..ca59ced 100644 --- a/src/routes/v1/documents.ts +++ b/src/routes/v1/documents.ts @@ -271,10 +271,10 @@ export function documentRoutes(db: DB, http?: HttpTransport): Hono { return c.json({ items: await coverCandidates(http ?? fetchTransport, title, q ? '' : (meta?.author ?? '')) }); }); - // Manual cover / print page count / title and author / language when lookups got it wrong. + // Manual cover / print page count / title and author / language / genres when lookups got it wrong. // Manual values stick (lookups only fill blanks, and device syncs don't replace an - // edited title or language); null clears cover or page count so it's looked up again, - // and language so the next device sync fills it. + // edited title or language); null clears cover, page count or genres so they're looked + // up again, and language so the next device sync fills it. app.put('/documents/:document/info', async (c) => { const param = c.req.param('document'); let body: unknown; @@ -290,12 +290,17 @@ export function documentRoutes(db: DB, http?: HttpTransport): Hono { const title = text(o.title); const author = text(o.author); const language = o.language === undefined || o.language === null ? o.language : languageTag(o.language); + // Same shape Hardcover's genres are stored in: a short list of short names. + const genres = Array.isArray(o.genres) ? [...new Set(o.genres.map(text))] : o.genres; + const genresOk = + genres === undefined || genres === null || + (Array.isArray(genres) && genres.length <= 10 && genres.every((g) => typeof g === 'string' && g.length > 0 && g.length <= 64)); // A title is required once edited; an empty author means "no author". const titleOk = title === undefined || (typeof title === 'string' && title.length > 0); const authorOk = author === undefined || typeof author === 'string'; const coverOk = cover === undefined || cover === null || (typeof cover === 'string' && /^https?:\/\/\S{1,2000}$/.test(cover)); const pagesOk = pages === undefined || pages === null || (Number.isInteger(pages) && (pages as number) > 0 && (pages as number) <= 100000); - if (!isValidDocument(param) || !coverOk || !pagesOk || !titleOk || !authorOk || (language === null && o.language !== null)) { + if (!isValidDocument(param) || !coverOk || !pagesOk || !titleOk || !authorOk || !genresOk || (language === null && o.language !== null)) { return kosyncError(c, 403, 2003, 'Invalid request'); } const user = c.get('user'); @@ -311,6 +316,14 @@ export function documentRoutes(db: DB, http?: HttpTransport): Hono { if (language !== undefined) { db.prepare('UPDATE documents SET language = ? WHERE user_id = ? AND document = ?').run(language as string | null, user.id, document); } + if (genres !== undefined) { + const list = genres as string[] | null; + // Clearing hands genres back to Hardcover: the next enrichment pass looks the book up again. + db.prepare( + `UPDATE documents SET genres = ?, genres_manual = ?, hc_checked_at = CASE WHEN ? THEN hc_checked_at ELSE NULL END + WHERE user_id = ? AND document = ?` + ).run(list?.length ? JSON.stringify(list) : null, list ? 1 : 0, list ? 1 : 0, user.id, document); + } if (title !== undefined || author !== undefined) { // New title/author: everything found with the old one is looked up again // (blank cover and page count, Hardcover details, automatic service matches). @@ -322,10 +335,11 @@ export function documentRoutes(db: DB, http?: HttpTransport): Hono { ).run((title as string | undefined) ?? null, author !== undefined ? 1 : 0, (author as string | undefined) ?? '', user.id, document); db.prepare("DELETE FROM connector_matches WHERE user_id = ? AND document = ? AND source IN ('auto', 'none')").run(user.id, document); } - const row = db.prepare('SELECT cover_url, page_count, title, author, language FROM documents WHERE user_id = ? AND document = ?').get(user.id, document) as { - cover_url: string | null; page_count: number | null; title: string | null; author: string | null; language: string | null; + const row = db.prepare('SELECT cover_url, page_count, title, author, language, genres FROM documents WHERE user_id = ? AND document = ?').get(user.id, document) as { + cover_url: string | null; page_count: number | null; title: string | null; author: string | null; language: string | null; genres: string | null; }; - return c.json({ document, cover_url: row.cover_url, page_count: row.page_count, title: row.title, author: row.author, language: row.language }); + return c.json({ document, cover_url: row.cover_url, page_count: row.page_count, title: row.title, author: row.author, language: row.language, + genres: row.genres ? (JSON.parse(row.genres) as string[]) : [] }); }); app.get('/documents', (c) => { diff --git a/test/manual-genres.test.ts b/test/manual-genres.test.ts new file mode 100644 index 0000000..8eeda61 --- /dev/null +++ b/test/manual-genres.test.ts @@ -0,0 +1,76 @@ +import { beforeEach, describe, expect, it } from 'vitest'; +import type { HttpTransport } from '../src/connectors/types.js'; +import { documentInfo } from '../src/models/cover.js'; +import { resetHardcoverThrottle } from '../src/models/hardcover-catalog.js'; +import { DOC, makeTestApp, registerUser } from './helpers.js'; + +// Hardcover finds the book with its own genres; the free cover sources find nothing. +const hardcover: HttpTransport = async (url) => { + const hit = { id: '41398', slug: 'foundryside', title: 'Foundryside', author_names: ['Robert Jackson Bennett'], genres: ['Fantasy', 'Magic'] }; + const body = url.includes('hardcover') ? { data: { search: { results: { hits: [{ document: hit }] } } } } : {}; + return { status: 200, text: async () => JSON.stringify(body), json: async () => body }; +}; + +beforeEach(() => resetHardcoverThrottle()); + +describe('manual genres', () => { + it('sticks over Hardcover lookups and clears back to them', async () => { + const { app, db } = makeTestApp(); + const { username, headers } = await registerUser(app); + await app.request('/syncs/progress', { + method: 'PUT', + headers, + body: JSON.stringify({ document: DOC, progress: '/body/DocFragment[2]', percentage: 0.2, device: 'X4', device_id: 'x4', + metadata: { title: 'Foundryside', authors: 'Robert Jackson Bennett' } }), + }); + const { id } = db.prepare('SELECT id FROM users WHERE username = ?').get(username) as { id: number }; + const put = (body: unknown) => app.request(`/api/v1/documents/${DOC}/info`, { method: 'PUT', headers, body: JSON.stringify(body) }); + const listed = async () => ((await (await app.request('/api/v1/progress', { headers })).json()) as { items: { genres: string[] }[] }).items[0].genres; + const enrich = () => documentInfo(db, id, DOC, hardcover, { HARDCOVER_API_KEY: 'k' }, { hardcover: true }); + + expect((await put({ genres: 'Fantasy' })).status).toBe(403); + expect((await put({ genres: [''] })).status).toBe(403); + expect((await put({ genres: ['x'.repeat(65)] })).status).toBe(403); + expect((await put({ genres: Array.from({ length: 11 }, (_, i) => `g${i}`) })).status).toBe(403); + + // Trimmed and de-duplicated, and back in the book list. + expect(await (await put({ genres: [' Heist ', 'Heist', 'Fantasy'] })).json()).toMatchObject({ genres: ['Heist', 'Fantasy'] }); + expect(await listed()).toEqual(['Heist', 'Fantasy']); + + // Hardcover's lookup fills the other details but leaves the genres alone. + await enrich(); + expect(await listed()).toEqual(['Heist', 'Fantasy']); + expect(db.prepare('SELECT hc_slug FROM documents WHERE document = ?').get(DOC)).toEqual({ hc_slug: 'foundryside' }); + + // null hands them back: the next lookup brings Hardcover's genres. + expect(await (await put({ genres: null })).json()).toMatchObject({ genres: [] }); + await enrich(); + expect(await listed()).toEqual(['Fantasy', 'Magic']); + + // Other info edits don't touch manual genres. + await put({ genres: ['Heist'] }); + await put({ page_count: 500 }); + expect(await listed()).toEqual(['Heist']); + }); + + it('keeps manual genres through a merge, over Hardcover\'s', async () => { + const { app, db } = makeTestApp(); + const { headers } = await registerUser(app); + const OTHER = 'ffffffffffffffffffffffffffffffff'; + for (const document of [DOC, OTHER]) { + await app.request('/syncs/progress', { + method: 'PUT', + headers, + body: JSON.stringify({ document, progress: '/body/DocFragment[2]', percentage: 0.2, device: 'X4', device_id: 'x4', + metadata: { title: 'Foundryside', authors: 'Robert Jackson Bennett' } }), + }); + } + // The kept book only has Hardcover's genres; the merged-away one has some set by hand. + db.prepare('UPDATE documents SET genres = ? WHERE document = ?').run('["Fantasy"]', DOC); + await app.request(`/api/v1/documents/${OTHER}/info`, { method: 'PUT', headers, body: JSON.stringify({ genres: ['Heist'] }) }); + + const merge = await app.request('/api/v1/documents/merge', { method: 'POST', headers, body: JSON.stringify({ document: OTHER, into: DOC }) }); + expect(merge.status).toBe(200); + expect(db.prepare('SELECT genres, genres_manual FROM documents WHERE document = ?').get(DOC)).toEqual({ genres: '["Heist"]', genres_manual: 1 }); + }); +});