mirror of
https://github.com/bookorbit/bookorbit.git
synced 2026-11-03 00:11:37 -06:00
fix(metadata): preserve authors during metadata refresh
Filter provider contributors by author role and treat empty values as absent for non-destructive merge strategies while preserving explicit overwrite clears. Fixes #1001
This commit is contained in:
@@ -402,6 +402,154 @@ describe('MetadataFetchPipeline', () => {
|
||||
expect(sources.description).toBe(MetadataProviderKey.OPEN_LIBRARY);
|
||||
});
|
||||
|
||||
it('treats an empty array as not provided and falls back under overwriteIfProvided', async () => {
|
||||
const prefs = createPreferences((fields) => {
|
||||
fields.authors = {
|
||||
enabled: true,
|
||||
providers: [MetadataProviderKey.GOOGLE, MetadataProviderKey.OPEN_LIBRARY],
|
||||
mergeStrategy: 'overwriteIfProvided',
|
||||
};
|
||||
});
|
||||
|
||||
preferencesService.getGlobal.mockResolvedValue(prefs);
|
||||
resolver.resolve.mockReturnValue(prefs);
|
||||
resolver.withForwardCompatibility.mockReturnValue(prefs);
|
||||
registry.all.mockReturnValue([{ key: MetadataProviderKey.GOOGLE }, { key: MetadataProviderKey.OPEN_LIBRARY }] as never);
|
||||
fetchService.search.mockReturnValue(
|
||||
of(
|
||||
candidate(MetadataProviderKey.GOOGLE, 'g1', { authors: [] }),
|
||||
candidate(MetadataProviderKey.OPEN_LIBRARY, 'ol1', { authors: ['Fallback Author'] }),
|
||||
),
|
||||
);
|
||||
|
||||
const { resolved, sources } = await pipeline.runWithSources({ title: 'Query' }, { authors: ['Existing Author'] });
|
||||
|
||||
expect(resolved.authors).toEqual(['Fallback Author']);
|
||||
expect(sources.authors).toBe(MetadataProviderKey.OPEN_LIBRARY);
|
||||
});
|
||||
|
||||
it('does not clear an existing array when overwriteIfProvided providers return empty arrays', async () => {
|
||||
const prefs = createPreferences((fields) => {
|
||||
fields.authors = {
|
||||
enabled: true,
|
||||
providers: [MetadataProviderKey.GOOGLE, MetadataProviderKey.OPEN_LIBRARY],
|
||||
mergeStrategy: 'overwriteIfProvided',
|
||||
};
|
||||
});
|
||||
|
||||
preferencesService.getGlobal.mockResolvedValue(prefs);
|
||||
resolver.resolve.mockReturnValue(prefs);
|
||||
resolver.withForwardCompatibility.mockReturnValue(prefs);
|
||||
registry.all.mockReturnValue([{ key: MetadataProviderKey.GOOGLE }, { key: MetadataProviderKey.OPEN_LIBRARY }] as never);
|
||||
fetchService.search.mockReturnValue(
|
||||
of(candidate(MetadataProviderKey.GOOGLE, 'g1', { authors: [] }), candidate(MetadataProviderKey.OPEN_LIBRARY, 'ol1', { authors: [] })),
|
||||
);
|
||||
|
||||
const { resolved, sources } = await pipeline.runWithSources({ title: 'Query' }, { authors: ['Existing Author'] });
|
||||
|
||||
expect(resolved.authors).toBeUndefined();
|
||||
expect(sources.authors).toBeUndefined();
|
||||
});
|
||||
|
||||
it('falls back from an empty array under fillMissing when the existing field is empty', async () => {
|
||||
const prefs = createPreferences((fields) => {
|
||||
fields.authors = {
|
||||
enabled: true,
|
||||
providers: [MetadataProviderKey.GOOGLE, MetadataProviderKey.OPEN_LIBRARY],
|
||||
mergeStrategy: 'fillMissing',
|
||||
};
|
||||
});
|
||||
|
||||
preferencesService.getGlobal.mockResolvedValue(prefs);
|
||||
resolver.resolve.mockReturnValue(prefs);
|
||||
resolver.withForwardCompatibility.mockReturnValue(prefs);
|
||||
registry.all.mockReturnValue([{ key: MetadataProviderKey.GOOGLE }, { key: MetadataProviderKey.OPEN_LIBRARY }] as never);
|
||||
fetchService.search.mockReturnValue(
|
||||
of(
|
||||
candidate(MetadataProviderKey.GOOGLE, 'g1', { authors: [] }),
|
||||
candidate(MetadataProviderKey.OPEN_LIBRARY, 'ol1', { authors: ['Fallback Author'] }),
|
||||
),
|
||||
);
|
||||
|
||||
const { resolved, sources } = await pipeline.runWithSources({ title: 'Query' }, { authors: [] });
|
||||
|
||||
expect(resolved.authors).toEqual(['Fallback Author']);
|
||||
expect(sources.authors).toBe(MetadataProviderKey.OPEN_LIBRARY);
|
||||
});
|
||||
|
||||
it('treats a blank string as not provided and falls back under overwriteIfProvided', async () => {
|
||||
const prefs = createPreferences((fields) => {
|
||||
fields.description = {
|
||||
enabled: true,
|
||||
providers: [MetadataProviderKey.GOOGLE, MetadataProviderKey.OPEN_LIBRARY],
|
||||
mergeStrategy: 'overwriteIfProvided',
|
||||
};
|
||||
});
|
||||
|
||||
preferencesService.getGlobal.mockResolvedValue(prefs);
|
||||
resolver.resolve.mockReturnValue(prefs);
|
||||
resolver.withForwardCompatibility.mockReturnValue(prefs);
|
||||
registry.all.mockReturnValue([{ key: MetadataProviderKey.GOOGLE }, { key: MetadataProviderKey.OPEN_LIBRARY }] as never);
|
||||
fetchService.search.mockReturnValue(
|
||||
of(
|
||||
candidate(MetadataProviderKey.GOOGLE, 'g1', { description: ' ' }),
|
||||
candidate(MetadataProviderKey.OPEN_LIBRARY, 'ol1', { description: 'Fallback Description' }),
|
||||
),
|
||||
);
|
||||
|
||||
const { resolved, sources } = await pipeline.runWithSources({ title: 'Query' }, { description: 'Existing Description' });
|
||||
|
||||
expect(resolved.description).toBe('Fallback Description');
|
||||
expect(sources.description).toBe(MetadataProviderKey.OPEN_LIBRARY);
|
||||
});
|
||||
|
||||
it('preserves empty arrays as explicit clears under overwrite', async () => {
|
||||
const prefs = createPreferences((fields) => {
|
||||
fields.authors = {
|
||||
enabled: true,
|
||||
providers: [MetadataProviderKey.GOOGLE],
|
||||
mergeStrategy: 'overwrite',
|
||||
};
|
||||
});
|
||||
|
||||
preferencesService.getGlobal.mockResolvedValue(prefs);
|
||||
resolver.resolve.mockReturnValue(prefs);
|
||||
resolver.withForwardCompatibility.mockReturnValue(prefs);
|
||||
registry.all.mockReturnValue([{ key: MetadataProviderKey.GOOGLE }] as never);
|
||||
fetchService.search.mockReturnValue(of(candidate(MetadataProviderKey.GOOGLE, 'g1', { authors: [] })));
|
||||
|
||||
const { resolved, sources } = await pipeline.runWithSources({ title: 'Query' }, { authors: ['Existing Author'] });
|
||||
|
||||
expect(resolved.authors).toEqual([]);
|
||||
expect(sources.authors).toBe(MetadataProviderKey.GOOGLE);
|
||||
});
|
||||
|
||||
it('keeps zero and false provider values under overwriteIfProvided', async () => {
|
||||
const prefs = createPreferences((fields) => {
|
||||
fields.duration = {
|
||||
enabled: true,
|
||||
providers: [MetadataProviderKey.GOOGLE],
|
||||
mergeStrategy: 'overwriteIfProvided',
|
||||
};
|
||||
fields.abridged = {
|
||||
enabled: true,
|
||||
providers: [MetadataProviderKey.GOOGLE],
|
||||
mergeStrategy: 'overwriteIfProvided',
|
||||
};
|
||||
});
|
||||
|
||||
preferencesService.getGlobal.mockResolvedValue(prefs);
|
||||
resolver.resolve.mockReturnValue(prefs);
|
||||
resolver.withForwardCompatibility.mockReturnValue(prefs);
|
||||
registry.all.mockReturnValue([{ key: MetadataProviderKey.GOOGLE }] as never);
|
||||
fetchService.search.mockReturnValue(of(candidate(MetadataProviderKey.GOOGLE, 'g1', { durationSeconds: 0, abridged: false })));
|
||||
|
||||
const { resolved } = await pipeline.runWithSources({ title: 'Query' }, { duration: 60, abridged: true });
|
||||
|
||||
expect(resolved.duration).toBe(0);
|
||||
expect(resolved.abridged).toBe(false);
|
||||
});
|
||||
|
||||
it('resolves community ratings from every configured provider', async () => {
|
||||
const prefs = createPreferences((fields) => {
|
||||
for (const field of ALL_METADATA_FIELDS) {
|
||||
|
||||
@@ -312,6 +312,7 @@ export class MetadataFetchPipeline {
|
||||
|
||||
let value = this.extractField(candidate, field);
|
||||
if (value === undefined || value === null) continue;
|
||||
if (mergeStrategy !== 'overwrite' && this.isEmptyProviderValue(value)) continue;
|
||||
|
||||
if (field === 'cover') {
|
||||
const existingValue = existing[field];
|
||||
@@ -512,6 +513,11 @@ export class MetadataFetchPipeline {
|
||||
return false;
|
||||
}
|
||||
|
||||
private isEmptyProviderValue(value: unknown): boolean {
|
||||
if (typeof value === 'string') return value.trim().length === 0;
|
||||
return Array.isArray(value) && value.length === 0;
|
||||
}
|
||||
|
||||
private filterExistingComicMetadata(
|
||||
resolved: ComicMetadataFields | undefined,
|
||||
existing: Partial<Record<keyof ComicMetadataFields, unknown>> | null | undefined,
|
||||
|
||||
@@ -10,6 +10,7 @@ const baseDocument: HardcoverSearchDocument = {
|
||||
subtitle: '10th Anniversary Edition',
|
||||
description: 'A story about a wizard.',
|
||||
author_names: ['Patrick Rothfuss'],
|
||||
contributions: [{ author: { id: 1, name: 'Patrick Rothfuss' }, contribution: 'Author' }],
|
||||
isbns: ['9780756404079', '0756404079'],
|
||||
pages: 662,
|
||||
release_year: 2007,
|
||||
@@ -134,9 +135,24 @@ describe('mapSearchDocument', () => {
|
||||
expect(result.seriesIndex).toBeUndefined();
|
||||
});
|
||||
|
||||
it('returns empty authors array when author_names is absent', () => {
|
||||
const doc: HardcoverSearchDocument = { ...baseDocument, author_names: undefined };
|
||||
expect(mapSearchDocument(doc).authors).toEqual([]);
|
||||
it('omits authors when contribution data is absent', () => {
|
||||
const doc: HardcoverSearchDocument = { ...baseDocument, contributions: undefined };
|
||||
expect(mapSearchDocument(doc).authors).toBeUndefined();
|
||||
});
|
||||
|
||||
it('maps only author contributions from search documents', () => {
|
||||
const doc: HardcoverSearchDocument = {
|
||||
...baseDocument,
|
||||
author_names: ['Primary Author', 'Second Author', 'Audio Narrator', 'Book Translator'],
|
||||
contributions: [
|
||||
{ author: { id: 1, name: 'Primary Author' }, contribution: null },
|
||||
{ author: { id: 2, name: 'Second Author' }, contribution: ' AUTHOR ' },
|
||||
{ author: { id: 3, name: 'Audio Narrator' }, contribution: 'Narrator' },
|
||||
{ author: { id: 4, name: 'Book Translator' }, contribution: 'Translator' },
|
||||
],
|
||||
};
|
||||
|
||||
expect(mapSearchDocument(doc).authors).toEqual(['Primary Author', 'Second Author']);
|
||||
});
|
||||
|
||||
it('returns undefined coverUrl when image is absent', () => {
|
||||
@@ -363,6 +379,59 @@ describe('mapBookWithEditions', () => {
|
||||
expect(mapBookWithEditions(book)[0].authors).toEqual(['Book Author']);
|
||||
});
|
||||
|
||||
it('falls back to book authors when the edition has only non-author contributors', () => {
|
||||
const book: HardcoverBookWithEditions = {
|
||||
...baseBook,
|
||||
cached_contributors: [{ author: { id: 2, name: 'Book Author' }, contribution: 'Author' }],
|
||||
editions: [
|
||||
{
|
||||
...baseBook.editions![0],
|
||||
cached_contributors: [
|
||||
{ author: { id: 3, name: 'Audio Narrator' }, contribution: 'Narrator' },
|
||||
{ author: { id: 4, name: 'Book Translator' }, contribution: 'Translator' },
|
||||
],
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
expect(mapBookWithEditions(book)[0].authors).toEqual(['Book Author']);
|
||||
});
|
||||
|
||||
it('maps only author contributions from an edition', () => {
|
||||
const book: HardcoverBookWithEditions = {
|
||||
...baseBook,
|
||||
editions: [
|
||||
{
|
||||
...baseBook.editions![0],
|
||||
cached_contributors: [
|
||||
{ author: { id: 1, name: 'Primary Author' }, contribution: null },
|
||||
{ author: { id: 2, name: 'Second Author' }, contribution: 'author' },
|
||||
{ author: { id: 3, name: 'Audio Narrator' }, contribution: 'Narrator' },
|
||||
{ author: { id: 4, name: 'Book Translator' }, contribution: 'Translator' },
|
||||
{ author: { id: 5, name: 'Cover Artist' }, contribution: 'Cover Artist' },
|
||||
],
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
expect(mapBookWithEditions(book)[0].authors).toEqual(['Primary Author', 'Second Author']);
|
||||
});
|
||||
|
||||
it('omits authors when neither the edition nor the book has an author contribution', () => {
|
||||
const book: HardcoverBookWithEditions = {
|
||||
...baseBook,
|
||||
cached_contributors: [{ author: { id: 2, name: 'Book Translator' }, contribution: 'Translator' }],
|
||||
editions: [
|
||||
{
|
||||
...baseBook.editions![0],
|
||||
cached_contributors: [{ author: { id: 3, name: 'Audio Narrator' }, contribution: 'Narrator' }],
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
expect(mapBookWithEditions(book)[0].authors).toBeUndefined();
|
||||
});
|
||||
|
||||
it('falls back to book title when edition title is absent', () => {
|
||||
const book: HardcoverBookWithEditions = {
|
||||
...baseBook,
|
||||
|
||||
@@ -17,7 +17,14 @@ function parseDate(releaseYear: number | undefined | null, releaseDate: string |
|
||||
|
||||
function extractAuthorsFromContributors(contributors: HardcoverCachedContributor[] | undefined): string[] {
|
||||
if (!contributors) return [];
|
||||
return contributors.map((c) => c.author?.name).filter((n): n is string => n != null);
|
||||
return contributors
|
||||
.filter(
|
||||
(contributor) =>
|
||||
contributor.contribution == null ||
|
||||
(typeof contributor.contribution === 'string' && contributor.contribution.trim().toLowerCase() === 'author'),
|
||||
)
|
||||
.map((contributor) => contributor.author?.name)
|
||||
.filter((name): name is string => typeof name === 'string' && name.trim().length > 0);
|
||||
}
|
||||
|
||||
function pickIsbn(isbns: string[] | undefined): { isbn10?: string; isbn13?: string } {
|
||||
@@ -81,6 +88,7 @@ function normalizeCommunityRatingCount(value: number | undefined): number | unde
|
||||
|
||||
export function mapSearchDocument(doc: HardcoverSearchDocument): MetadataCandidate {
|
||||
const { isbn10, isbn13 } = pickIsbn(doc.isbns);
|
||||
const authors = extractAuthorsFromContributors(doc.contributions);
|
||||
const communityRating = normalizeCommunityRating(doc.rating);
|
||||
const communityRatingCount = normalizeCommunityRatingCount(doc.ratings_count);
|
||||
const { title, subtitle } = splitEmbeddedSubtitle(doc.title, doc.subtitle);
|
||||
@@ -91,7 +99,7 @@ export function mapSearchDocument(doc: HardcoverSearchDocument): MetadataCandida
|
||||
title,
|
||||
subtitle,
|
||||
description: doc.description,
|
||||
authors: doc.author_names ?? [],
|
||||
...(authors.length > 0 ? { authors } : {}),
|
||||
pageCount: doc.pages,
|
||||
publishedDate: parseDate(doc.release_year, doc.release_date),
|
||||
publishedYear: parseYear(doc.release_year, doc.release_date),
|
||||
@@ -126,7 +134,7 @@ function mapEdition(edition: HardcoverEdition, book: HardcoverBookWithEditions):
|
||||
title,
|
||||
subtitle,
|
||||
description: book.description,
|
||||
authors,
|
||||
...(authors.length > 0 ? { authors } : {}),
|
||||
publisher: edition.publisher?.name,
|
||||
language: edition.language?.code2,
|
||||
pageCount: isAudiobookEdition(edition) ? undefined : (edition.pages ?? book.pages),
|
||||
|
||||
@@ -22,6 +22,7 @@ export interface HardcoverSearchDocument {
|
||||
subtitle?: string;
|
||||
description?: string;
|
||||
author_names?: string[];
|
||||
contributions?: HardcoverCachedContributor[];
|
||||
isbns?: string[];
|
||||
pages?: number;
|
||||
release_date?: string;
|
||||
|
||||
Reference in New Issue
Block a user