Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions docs/API.md
Original file line number Diff line number Diff line change
Expand Up @@ -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=

Expand Down
2 changes: 2 additions & 0 deletions migrations/0033_manual_genres.sql
Original file line number Diff line number Diff line change
@@ -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;
4 changes: 2 additions & 2 deletions src/models/cover.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
8 changes: 6 additions & 2 deletions src/models/merge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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);
Expand Down
28 changes: 21 additions & 7 deletions src/routes/v1/documents.ts
Original file line number Diff line number Diff line change
Expand Up @@ -271,10 +271,10 @@ export function documentRoutes(db: DB, http?: HttpTransport): Hono<AppEnv> {
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;
Expand All @@ -290,12 +290,17 @@ export function documentRoutes(db: DB, http?: HttpTransport): Hono<AppEnv> {
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');
Expand All @@ -311,6 +316,14 @@ export function documentRoutes(db: DB, http?: HttpTransport): Hono<AppEnv> {
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);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
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).
Expand All @@ -322,10 +335,11 @@ export function documentRoutes(db: DB, http?: HttpTransport): Hono<AppEnv> {
).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) => {
Expand Down
76 changes: 76 additions & 0 deletions test/manual-genres.test.ts
Original file line number Diff line number Diff line change
@@ -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 });
});
});
Loading