mirror of
https://github.com/bookorbit/bookorbit.git
synced 2026-11-03 00:11:37 -06:00
fix(metadata): extract and write back comicvine id for comics (#842)
* fix(metadata): extract and write back comicvine id for comics ComicVine references were never read from a comic's embedded ComicInfo.xml on import, even though every other provider id (Goodreads, Amazon, Hardcover, Google Books, OpenLibrary, Kobo) already round-trips through the same <Web>/Notes fields. This affected Book Dock import, direct upload, and library scanning alike, since they share the same extractor. Write-back had the same gap: a comicvineId resolved via search/auto-fill was never persisted back into the file. Closes #840 * fix(metadata): keep comicvine id off the contested Web slot and source-check notes ComicInfo has a single Web element and buildComicInfoXml writes the first provider id in COMIC_INFO_PROVIDER_ID_KEYS order into it. The other six providers are also in COMIC_INFO_MANAGED_NOTES_KEYS, so losing that slot costs them nothing; comicvineId had no second channel, so a write-back for a book that also carried a goodreads/amazon/hardcover/google/openlibrary/kobo id dropped the id from the file entirely and clobbered any ComicVine URL that was already there. Add comicvineId to the managed notes keys, which also makes the existing managed-note read precedence reachable from our own writes. ComicTagger emits "[Issue ID N]" for whichever talker produced the tag, naming the source separately as "using info from <source>". Matching the bare marker attributed Metron and other non-ComicVine issue ids to ComicVine and wrote them back out as fabricated comicvine.gamespot.com URLs. Require the Comic Vine source in the same sentence. Also anchor the Web URL scan to a path segment so a segment ending in 4000-, such as "14000-777", is no longer read as an issue id. --------- Co-authored-by: neon <18559658+neonsolstice@users.noreply.github.com>
This commit is contained in:
@@ -31,6 +31,7 @@ export const COMMON_PROVIDER_BOOK_FILE_WRITE_FIELDS = [
|
||||
] as const;
|
||||
|
||||
export const COMIC_BOOK_FILE_WRITE_FIELDS = [
|
||||
"comicvineId",
|
||||
"comicIssueNumber",
|
||||
"comicVolumeName",
|
||||
"comicPencillers",
|
||||
@@ -84,6 +85,7 @@ export const BOOK_FILE_WRITE_FIELD_LABELS = {
|
||||
koboId: "Kobo ID",
|
||||
lubimyczytacId: "LubimyCzytac ID",
|
||||
aladinId: "Aladin ID",
|
||||
comicvineId: "ComicVine ID",
|
||||
comicIssueNumber: "Issue number",
|
||||
comicVolumeName: "Volume",
|
||||
comicPencillers: "Pencillers",
|
||||
|
||||
@@ -31,6 +31,7 @@ export const COMIC_INFO_PROVIDER_ID_KEYS = [
|
||||
'googleBooksId',
|
||||
'openLibraryId',
|
||||
'koboId',
|
||||
'comicvineId',
|
||||
] as const satisfies readonly BookWritePayloadKey[];
|
||||
|
||||
export const COMIC_INFO_MANAGED_NOTES_KEYS = [
|
||||
@@ -46,6 +47,7 @@ export const COMIC_INFO_MANAGED_NOTES_KEYS = [
|
||||
'koboId',
|
||||
'lubimyczytacId',
|
||||
'aladinId',
|
||||
'comicvineId',
|
||||
] as const satisfies readonly BookWritePayloadKey[];
|
||||
|
||||
type ComicInfoProviderKey = (typeof COMIC_INFO_PROVIDER_ID_KEYS)[number];
|
||||
@@ -57,6 +59,7 @@ export const COMIC_INFO_PROVIDER_WEB_URL_BUILDERS: Record<ComicInfoProviderKey,
|
||||
googleBooksId: (id: string) => `https://books.google.com/books?id=${id}`,
|
||||
openLibraryId: (id: string) => `https://openlibrary.org/works/${id}`,
|
||||
koboId: (id: string) => `https://www.kobo.com/us/en/ebook/${id}`,
|
||||
comicvineId: (id: string) => `https://comicvine.gamespot.com/-/4000-${id}/`,
|
||||
};
|
||||
|
||||
export const EPUB_PROVIDER_IDENTIFIER_SCHEMES = {
|
||||
|
||||
@@ -100,6 +100,7 @@ describe('FileWriteRepository', () => {
|
||||
openLibraryId: 'ol',
|
||||
ranobedbId: 'rn',
|
||||
koboId: 'kb',
|
||||
comicvineId: 'cv',
|
||||
lubimyczytacId: 'lc',
|
||||
aladinId: 'al',
|
||||
itunesId: 'it',
|
||||
|
||||
@@ -218,6 +218,7 @@ export class FileWriteRepository {
|
||||
openLibraryId: meta.openLibraryId,
|
||||
ranobedbId: meta.ranobedbId,
|
||||
koboId: meta.koboId,
|
||||
comicvineId: meta.comicvineId,
|
||||
lubimyczytacId: meta.lubimyczytacId,
|
||||
aladinId: meta.aladinId,
|
||||
itunesId: meta.itunesId,
|
||||
|
||||
@@ -157,6 +157,23 @@ describe('buildComicInfoXml', () => {
|
||||
expect(notes).not.toContain('[bookorbit:subtitle] old');
|
||||
});
|
||||
|
||||
it('writes comicvineId into Web using the canonical 4000-<id> issue URL', () => {
|
||||
const xml = buildComicInfoXml(null, { comicvineId: '140529' }, new Set(['comicvineId']));
|
||||
|
||||
const parsed = parser.parse(xml) as { ComicInfo: Record<string, string> };
|
||||
|
||||
expect(parsed.ComicInfo.Web).toBe('https://comicvine.gamespot.com/-/4000-140529/');
|
||||
});
|
||||
|
||||
it('keeps comicvineId in Notes when another provider id owns the single Web slot', () => {
|
||||
const xml = buildComicInfoXml(null, { goodreadsId: '111', comicvineId: '140529' }, new Set(['goodreadsId', 'comicvineId']));
|
||||
|
||||
const parsed = parser.parse(xml) as { ComicInfo: Record<string, string> };
|
||||
|
||||
expect(parsed.ComicInfo.Web).toBe('https://www.goodreads.com/book/show/111');
|
||||
expect(parsed.ComicInfo.Notes).toContain('[bookorbit:comicvineId] 140529');
|
||||
});
|
||||
|
||||
it('does not overwrite provider-derived Web/Notes when provider fields are excluded from field mask', () => {
|
||||
const existing = `<?xml version="1.0"?><ComicInfo><Web>https://www.goodreads.com/book/show/111</Web><Notes>[bookorbit:goodreadsId] 111</Notes></ComicInfo>`;
|
||||
|
||||
|
||||
@@ -28,6 +28,7 @@ export interface BookWritePayload {
|
||||
koboId?: string | null;
|
||||
lubimyczytacId?: string | null;
|
||||
aladinId?: string | null;
|
||||
comicvineId?: string | null;
|
||||
comicIssueNumber?: string | null;
|
||||
comicVolumeName?: string | null;
|
||||
comicPencillers?: string[];
|
||||
|
||||
@@ -50,6 +50,7 @@ export class ComicFormatExtractor implements FormatExtractor {
|
||||
openLibraryId: comicMetadata?.openLibraryId ?? null,
|
||||
ranobedbId: comicMetadata?.ranobedbId ?? null,
|
||||
koboId: comicMetadata?.koboId ?? null,
|
||||
comicvineId: comicMetadata?.comicvineId ?? null,
|
||||
lubimyczytacId: comicMetadata?.lubimyczytacId ?? null,
|
||||
aladinId: comicMetadata?.aladinId ?? null,
|
||||
itunesId: comicMetadata?.itunesId ?? null,
|
||||
|
||||
@@ -24,6 +24,7 @@ export interface ParsedBookData {
|
||||
openLibraryId?: string | null;
|
||||
ranobedbId?: string | null;
|
||||
koboId?: string | null;
|
||||
comicvineId?: string | null;
|
||||
lubimyczytacId?: string | null;
|
||||
aladinId?: string | null;
|
||||
itunesId?: string | null;
|
||||
|
||||
@@ -379,6 +379,7 @@ describe('metadata format extractors', () => {
|
||||
publishedYear: 2014,
|
||||
authors: [],
|
||||
genres: [],
|
||||
comicvineId: null,
|
||||
}),
|
||||
);
|
||||
});
|
||||
@@ -396,6 +397,7 @@ describe('metadata format extractors', () => {
|
||||
genres: ['Comics'],
|
||||
tags: ['Superhero'],
|
||||
ranobedbId: 'comic-ranobe',
|
||||
comicvineId: '140529',
|
||||
comicMetadata: { issueNumber: '55', volumeName: 'Batman' },
|
||||
});
|
||||
mockExtractCbrCover.mockResolvedValue(null);
|
||||
@@ -405,6 +407,7 @@ describe('metadata format extractors', () => {
|
||||
title: 'Batman',
|
||||
genres: ['Comics'],
|
||||
ranobedbId: 'comic-ranobe',
|
||||
comicvineId: '140529',
|
||||
comicMetadata: { issueNumber: '55', volumeName: 'Batman' },
|
||||
}),
|
||||
);
|
||||
|
||||
@@ -235,6 +235,93 @@ describe('extractCbzMetadata', () => {
|
||||
expect(r?.googleBooksId).toBe('google-1');
|
||||
expect(r?.hardcoverEditionId).toBe('8941973');
|
||||
});
|
||||
|
||||
describe('ComicVine ID extraction', () => {
|
||||
it('extracts comicvineId from a Web field containing the canonical 4000-<id> issue URL', async () => {
|
||||
const xml = `<ComicInfo><Web>https://comicvine.gamespot.com/amazing-series-1/4000-140529/</Web></ComicInfo>`;
|
||||
mockZipFile(buildZipWithComicInfo(xml));
|
||||
|
||||
const r = await extractCbzMetadata('/book.cbz');
|
||||
expect(r?.comicvineId).toBe('140529');
|
||||
});
|
||||
|
||||
it('extracts comicvineId when Web holds multiple space-separated URLs', async () => {
|
||||
const xml = `<ComicInfo><Web>https://www.example.com/other https://comicvine.gamespot.com/amazing-series-1/4000-140529/</Web></ComicInfo>`;
|
||||
mockZipFile(buildZipWithComicInfo(xml));
|
||||
|
||||
const r = await extractCbzMetadata('/book.cbz');
|
||||
expect(r?.comicvineId).toBe('140529');
|
||||
});
|
||||
|
||||
it('falls back to the "[Issue ID N]" convention in Notes when Web has no ComicVine URL', async () => {
|
||||
const xml = `<ComicInfo>
|
||||
<Notes>Tagged with ComicTagger 1.3.2a5 using info from Comic Vine on 2022-04-16. [Issue ID 140529]</Notes>
|
||||
</ComicInfo>`;
|
||||
mockZipFile(buildZipWithComicInfo(xml));
|
||||
|
||||
const r = await extractCbzMetadata('/book.cbz');
|
||||
expect(r?.comicvineId).toBe('140529');
|
||||
});
|
||||
|
||||
it('ignores the Notes fallback when ComicTagger sourced the issue id from another provider', async () => {
|
||||
const xml = `<ComicInfo>
|
||||
<Notes>Tagged with ComicTagger 2.10.0 using info from Metron on 2024-01-02. [Issue ID 55123]</Notes>
|
||||
</ComicInfo>`;
|
||||
mockZipFile(buildZipWithComicInfo(xml));
|
||||
|
||||
const r = await extractCbzMetadata('/book.cbz');
|
||||
expect(r?.comicvineId).toBeNull();
|
||||
});
|
||||
|
||||
it('prefers the Web field over the Notes fallback when both are present', async () => {
|
||||
const xml = `<ComicInfo>
|
||||
<Web>https://comicvine.gamespot.com/amazing-series-1/4000-1/</Web>
|
||||
<Notes>Tagged with ComicTagger 1.3.2a5 using info from Comic Vine on 2022-04-16. [Issue ID 999999]</Notes>
|
||||
</ComicInfo>`;
|
||||
mockZipFile(buildZipWithComicInfo(xml));
|
||||
|
||||
const r = await extractCbzMetadata('/book.cbz');
|
||||
expect(r?.comicvineId).toBe('1');
|
||||
});
|
||||
|
||||
it('prefers a managed [bookorbit:comicvineId] note over both Web and the Notes fallback', async () => {
|
||||
const xml = `<ComicInfo>
|
||||
<Web>https://comicvine.gamespot.com/amazing-series-1/4000-1/</Web>
|
||||
<Notes>
|
||||
[bookorbit:comicvineId] 42
|
||||
Tagged with ComicTagger 1.3.2a5 using info from Comic Vine on 2022-04-16. [Issue ID 999999]
|
||||
</Notes>
|
||||
</ComicInfo>`;
|
||||
mockZipFile(buildZipWithComicInfo(xml));
|
||||
|
||||
const r = await extractCbzMetadata('/book.cbz');
|
||||
expect(r?.comicvineId).toBe('42');
|
||||
});
|
||||
|
||||
it('returns null when neither Web nor Notes reference ComicVine', async () => {
|
||||
const xml = `<ComicInfo><Title>No ComicVine reference</Title></ComicInfo>`;
|
||||
mockZipFile(buildZipWithComicInfo(xml));
|
||||
|
||||
const r = await extractCbzMetadata('/book.cbz');
|
||||
expect(r?.comicvineId).toBeNull();
|
||||
});
|
||||
|
||||
it('does not confuse a ComicVine volume URL (4050-) with an issue URL (4000-)', async () => {
|
||||
const xml = `<ComicInfo><Web>https://comicvine.gamespot.com/amazing-series/4050-9999/</Web></ComicInfo>`;
|
||||
mockZipFile(buildZipWithComicInfo(xml));
|
||||
|
||||
const r = await extractCbzMetadata('/book.cbz');
|
||||
expect(r?.comicvineId).toBeNull();
|
||||
});
|
||||
|
||||
it('only matches 4000- at the start of a path segment', async () => {
|
||||
const xml = `<ComicInfo><Web>https://comicvine.gamespot.com/amazing-series/14000-777/</Web></ComicInfo>`;
|
||||
mockZipFile(buildZipWithComicInfo(xml));
|
||||
|
||||
const r = await extractCbzMetadata('/book.cbz');
|
||||
expect(r?.comicvineId).toBeNull();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('ComicBookInfo/1.0 JSON comment fallback', () => {
|
||||
|
||||
@@ -45,6 +45,7 @@ export interface ParsedCbzMetadata {
|
||||
openLibraryId: string | null;
|
||||
ranobedbId: string | null;
|
||||
koboId: string | null;
|
||||
comicvineId: string | null;
|
||||
lubimyczytacId: string | null;
|
||||
aladinId: string | null;
|
||||
itunesId: string | null;
|
||||
@@ -106,6 +107,20 @@ function parseProviderIdsFromWebUrl(
|
||||
return {};
|
||||
}
|
||||
|
||||
function parseComicVineIdFromWebUrl(webUrl: string | null): string | null {
|
||||
if (!webUrl) return null;
|
||||
// Web may hold multiple space-separated URLs, so scan rather than match a fixed prefix.
|
||||
// 4000- must start a path segment, otherwise a segment merely ending in it, such as "14000-777", matches too.
|
||||
return /comicvine\.gamespot\.com\/(?:[^\s]*\/)?4000-(\d+)/i.exec(webUrl)?.[1] ?? null;
|
||||
}
|
||||
|
||||
function parseComicVineIdFromNotes(notes: string | null): string | null {
|
||||
if (!notes) return null;
|
||||
// ComicTagger writes "using info from <source> on <date>. [Issue ID 140529]" for every metadata
|
||||
// source it supports, so the id only belongs to Comic Vine when that source names it.
|
||||
return /using info from Comic\s*Vine\b[^[]*\[Issue ID (\d+)\]/i.exec(notes)?.[1] ?? null;
|
||||
}
|
||||
|
||||
function parseCbxRating(value: string | null): number | null {
|
||||
if (!value) return null;
|
||||
const parsed = Number(value);
|
||||
@@ -145,8 +160,11 @@ function parseComicInfoXml(xmlBuf: Buffer): ParsedCbzMetadata | null {
|
||||
year !== null && month !== null && day !== null
|
||||
? (parsePublishedDateKey(`${year}-${String(Math.floor(month)).padStart(2, '0')}-${String(Math.floor(day)).padStart(2, '0')}`) ?? null)
|
||||
: null;
|
||||
const managedNotes = parseProjectxManagedNotes(str('Notes'));
|
||||
const providerIdsFromWeb = parseProviderIdsFromWebUrl(str('Web'));
|
||||
const notesText = str('Notes');
|
||||
const webText = str('Web');
|
||||
const managedNotes = parseProjectxManagedNotes(notesText);
|
||||
const providerIdsFromWeb = parseProviderIdsFromWebUrl(webText);
|
||||
const comicvineId = managedNotes.get('comicvineId') ?? parseComicVineIdFromWebUrl(webText) ?? parseComicVineIdFromNotes(notesText) ?? null;
|
||||
|
||||
const genres = splitDelimited(str('Genre'));
|
||||
const tags = splitDelimited(str('Tags'));
|
||||
@@ -201,6 +219,7 @@ function parseComicInfoXml(xmlBuf: Buffer): ParsedCbzMetadata | null {
|
||||
openLibraryId: managedNotes.get('openLibraryId') ?? providerIdsFromWeb.openLibraryId ?? null,
|
||||
ranobedbId: managedNotes.get('ranobedbId') ?? null,
|
||||
koboId: managedNotes.get('koboId') ?? providerIdsFromWeb.koboId ?? null,
|
||||
comicvineId,
|
||||
lubimyczytacId: managedNotes.get('lubimyczytacId') ?? null,
|
||||
aladinId: managedNotes.get('aladinId') ?? null,
|
||||
itunesId: null,
|
||||
@@ -271,6 +290,7 @@ function parseComicBookInfoJson(comment: string): ParsedCbzMetadata | null {
|
||||
openLibraryId: null,
|
||||
ranobedbId: null,
|
||||
koboId: null,
|
||||
comicvineId: null,
|
||||
lubimyczytacId: null,
|
||||
aladinId: null,
|
||||
itunesId: null,
|
||||
|
||||
@@ -86,7 +86,9 @@ import { mkdir, readFile, readdir, rm, writeFile } from 'fs/promises';
|
||||
import { Logger } from '@nestjs/common';
|
||||
|
||||
import { authors, bookAuthors, bookGenres, bookMetadata, books, bookTags, genres, tags } from '../../db/schema';
|
||||
import { extractCbzMetadata } from './lib/cbz-metadata';
|
||||
import { generateThumbnail, imageExt } from './lib/cover';
|
||||
import { extractCbzCover } from './lib/cover-cbz';
|
||||
import { extractEpubCover } from './lib/cover-epub';
|
||||
import { extractEpubMetadata } from './lib/epub';
|
||||
import { parseBookFilename } from './lib/filename-parser';
|
||||
@@ -109,6 +111,8 @@ const mockParseMobiFile = parseMobiFile as MockedFunction<typeof parseMobiFile>;
|
||||
const mockParsePdfFile = parsePdfFile as MockedFunction<typeof parsePdfFile>;
|
||||
const mockExtractEpubCover = extractEpubCover as MockedFunction<typeof extractEpubCover>;
|
||||
const mockExtractEpubMetadata = extractEpubMetadata as MockedFunction<typeof extractEpubMetadata>;
|
||||
const mockExtractCbzMetadata = extractCbzMetadata as MockedFunction<typeof extractCbzMetadata>;
|
||||
const mockExtractCbzCover = extractCbzCover as MockedFunction<typeof extractCbzCover>;
|
||||
const mockExtractAudioMetadata = extractAudioMetadata as MockedFunction<typeof extractAudioMetadata>;
|
||||
const mockParseAudioDuration = parseAudioDuration as MockedFunction<typeof parseAudioDuration>;
|
||||
|
||||
@@ -176,6 +180,8 @@ describe('MetadataService', () => {
|
||||
mockParseBookFilename.mockReturnValue({ title: 'Fallback Title', publishedYear: 2001 });
|
||||
mockParseMobiFile.mockResolvedValue(null);
|
||||
mockParsePdfFile.mockResolvedValue(null);
|
||||
mockExtractCbzMetadata.mockResolvedValue(null);
|
||||
mockExtractCbzCover.mockResolvedValue(null);
|
||||
mockExtractEpubMetadata.mockResolvedValue(null);
|
||||
mockExtractEpubCover.mockResolvedValue(null);
|
||||
mockExtractAudioMetadata.mockResolvedValue({
|
||||
@@ -620,6 +626,53 @@ describe('MetadataService', () => {
|
||||
expect(embedder.embedBook).toHaveBeenCalledWith(22);
|
||||
});
|
||||
|
||||
it('extractAndSave(cbz) persists comicvineId parsed from the embedded ComicInfo.xml', async () => {
|
||||
const { db, updateSet } = makeDb();
|
||||
const service = makeService(db);
|
||||
vi.spyOn(service, 'replaceAuthors').mockResolvedValue(undefined);
|
||||
vi.spyOn(service, 'replaceGenres').mockResolvedValue(undefined);
|
||||
|
||||
mockExtractCbzMetadata.mockResolvedValue({
|
||||
title: 'Amazing Series',
|
||||
subtitle: null,
|
||||
seriesName: 'Amazing Series',
|
||||
seriesIndex: 1,
|
||||
description: null,
|
||||
publisher: null,
|
||||
publishedDate: null,
|
||||
publishedYear: null,
|
||||
language: null,
|
||||
pageCount: null,
|
||||
rating: null,
|
||||
isbn10: null,
|
||||
isbn13: null,
|
||||
authors: [],
|
||||
genres: [],
|
||||
tags: [],
|
||||
googleBooksId: null,
|
||||
goodreadsId: null,
|
||||
amazonId: null,
|
||||
hardcoverId: null,
|
||||
hardcoverEditionId: null,
|
||||
openLibraryId: null,
|
||||
ranobedbId: null,
|
||||
koboId: null,
|
||||
comicvineId: '140529',
|
||||
lubimyczytacId: null,
|
||||
aladinId: null,
|
||||
itunesId: null,
|
||||
comicMetadata: null,
|
||||
});
|
||||
|
||||
await service.extractAndSave(23, '/tmp/book.cbz', 'cbz');
|
||||
|
||||
expect(updateSet).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
comicvineId: '140529',
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('extractAndSave(mobi) ignores malformed publishedDate values from providers', async () => {
|
||||
const { db, updateSet } = makeDb();
|
||||
const service = makeService(db);
|
||||
|
||||
@@ -651,6 +651,7 @@ export class MetadataService {
|
||||
openLibraryId: data.openLibraryId,
|
||||
ranobedbId: data.ranobedbId,
|
||||
koboId: data.koboId,
|
||||
comicvineId: data.comicvineId,
|
||||
lubimyczytacId: data.lubimyczytacId,
|
||||
aladinId: data.aladinId,
|
||||
itunesId: data.itunesId,
|
||||
@@ -688,6 +689,7 @@ export class MetadataService {
|
||||
if (filtered.openLibraryId !== undefined) scalarFields.openLibraryId = filtered.openLibraryId;
|
||||
if (filtered.ranobedbId !== undefined) scalarFields.ranobedbId = filtered.ranobedbId;
|
||||
if (filtered.koboId !== undefined) scalarFields.koboId = filtered.koboId;
|
||||
if (filtered.comicvineId !== undefined) scalarFields.comicvineId = filtered.comicvineId;
|
||||
if (filtered.lubimyczytacId !== undefined) scalarFields.lubimyczytacId = filtered.lubimyczytacId;
|
||||
if (filtered.aladinId !== undefined) scalarFields.aladinId = filtered.aladinId;
|
||||
if (filtered.itunesId !== undefined) scalarFields.itunesId = filtered.itunesId;
|
||||
|
||||
Reference in New Issue
Block a user