mirror of
https://github.com/bookorbit/bookorbit.git
synced 2026-11-03 00:11:37 -06:00
fix(metadata): populate ComicVine issue publishers
Fetch and cache linked ComicVine volumes so issue candidates expose publisher metadata. Preserve candidates when enrichment fails or is throttled. Fixes #1102
This commit is contained in:
@@ -261,6 +261,95 @@ describe('ComicVineClient', () => {
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// getVolumeById
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('getVolumeById', () => {
|
||||
const volume = { id: 796, name: 'Batman', publisher: { id: 10, name: 'DC Comics' } };
|
||||
|
||||
it('requests the ComicVine volume resource with publisher included', async () => {
|
||||
mockFetch.mockResolvedValue(makeOkResponse(volume, false));
|
||||
|
||||
const pending = client.getVolumeById(796, 'key');
|
||||
await vi.runAllTimersAsync();
|
||||
|
||||
expect(await pending).toEqual(volume);
|
||||
const url = new URL(String(mockFetch.mock.calls[0]?.[0]));
|
||||
expect(url.pathname).toBe('/api/volume/4050-796/');
|
||||
expect(url.searchParams.get('field_list')?.split(',')).toContain('publisher');
|
||||
});
|
||||
|
||||
it('returns a cached volume without another request', async () => {
|
||||
mockFetch.mockResolvedValue(makeOkResponse(volume, false));
|
||||
|
||||
const first = client.getVolumeById(796, 'key');
|
||||
await vi.runAllTimersAsync();
|
||||
await first;
|
||||
|
||||
const second = client.getVolumeById(796, 'key');
|
||||
await vi.runAllTimersAsync();
|
||||
|
||||
expect(await second).toEqual(volume);
|
||||
expect(mockFetch).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('reuses a volume cached by a structured volume search', async () => {
|
||||
mockFetch.mockResolvedValue(makeOkResponse([volume]));
|
||||
|
||||
const search = client.searchVolumes('Batman', 'key');
|
||||
await vi.runAllTimersAsync();
|
||||
await search;
|
||||
|
||||
const result = await client.getVolumeById(796, 'key');
|
||||
|
||||
expect(result).toEqual(volume);
|
||||
expect(mockFetch).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('refreshes a cached volume after the TTL expires', async () => {
|
||||
mockFetch.mockResolvedValueOnce(makeOkResponse(volume, false));
|
||||
|
||||
const first = client.getVolumeById(796, 'key');
|
||||
await vi.runAllTimersAsync();
|
||||
await first;
|
||||
vi.advanceTimersByTime(10 * 60 * 1000 + 1);
|
||||
|
||||
const updated = { ...volume, publisher: { id: 11, name: 'Updated Comics' } };
|
||||
mockFetch.mockResolvedValueOnce(makeOkResponse(updated, false));
|
||||
const second = client.getVolumeById(796, 'key');
|
||||
await vi.runAllTimersAsync();
|
||||
|
||||
expect(await second).toEqual(updated);
|
||||
expect(mockFetch).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('returns null on a non-ok response', async () => {
|
||||
mockFetch.mockResolvedValue(makeErrorResponse(404));
|
||||
|
||||
const pending = client.getVolumeById(796, 'key');
|
||||
await vi.runAllTimersAsync();
|
||||
|
||||
expect(await pending).toBeNull();
|
||||
});
|
||||
|
||||
it('rejects a malformed volume id without making a request', async () => {
|
||||
await expect(client.getVolumeById(Number.NaN, 'key')).resolves.toBeNull();
|
||||
await expect(client.getVolumeById(-1, 'key')).resolves.toBeNull();
|
||||
|
||||
expect(mockFetch).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('throws ProviderThrottleError on 420', async () => {
|
||||
mockFetch.mockResolvedValue(make420Response());
|
||||
|
||||
const pending = client.getVolumeById(796, 'key');
|
||||
const assertion = expect(pending).rejects.toThrow(ProviderThrottleError);
|
||||
await vi.runAllTimersAsync();
|
||||
await assertion;
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// searchIssuesInVolume
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
@@ -24,6 +24,11 @@ interface CachedVolumes {
|
||||
expiresAt: number;
|
||||
}
|
||||
|
||||
interface CachedVolume {
|
||||
data: ComicVineVolume;
|
||||
expiresAt: number;
|
||||
}
|
||||
|
||||
class RateLimiter {
|
||||
private nextAllowedTime = 0;
|
||||
private readonly timestamps: number[] = [];
|
||||
@@ -60,6 +65,7 @@ export class ComicVineClient {
|
||||
private readonly logger = new Logger(ComicVineClient.name);
|
||||
private readonly rateLimiter = new RateLimiter();
|
||||
private readonly volumeCache = new Map<string, CachedVolumes>();
|
||||
private readonly volumeByIdCache = new Map<number, CachedVolume>();
|
||||
|
||||
windowResetMs(): number {
|
||||
return this.rateLimiter.timeUntilWindowResetMs();
|
||||
@@ -81,10 +87,28 @@ export class ComicVineClient {
|
||||
const data = await this.get<ComicVineVolume[]>(url, signal);
|
||||
if (data) {
|
||||
this.cacheVolumes(cacheKey, data);
|
||||
for (const volume of data) this.cacheVolume(volume);
|
||||
}
|
||||
return data ?? [];
|
||||
}
|
||||
|
||||
async getVolumeById(volumeId: number, apiKey: string, signal?: AbortSignal): Promise<ComicVineVolume | null> {
|
||||
if (!Number.isSafeInteger(volumeId) || volumeId <= 0) return null;
|
||||
|
||||
const cached = this.volumeByIdCache.get(volumeId);
|
||||
if (cached && cached.expiresAt > Date.now()) {
|
||||
return cached.data;
|
||||
}
|
||||
|
||||
const url = this.buildUrl(`/volume/4050-${volumeId}/`, apiKey, {
|
||||
field_list: VOLUME_FIELDS,
|
||||
});
|
||||
|
||||
const data = await this.get<ComicVineVolume>(url, signal);
|
||||
if (data) this.cacheVolume(data);
|
||||
return data;
|
||||
}
|
||||
|
||||
async searchIssuesInVolume(volumeId: number, issueNumber: string, apiKey: string, signal?: AbortSignal): Promise<ComicVineIssue[]> {
|
||||
const url = this.buildUrl('/issues/', apiKey, {
|
||||
filter: `volume:${volumeId},issue_number:${issueNumber}`,
|
||||
@@ -168,4 +192,14 @@ export class ComicVineClient {
|
||||
}
|
||||
this.volumeCache.set(key, { data, expiresAt: Date.now() + VOLUME_CACHE_TTL_MS });
|
||||
}
|
||||
|
||||
private cacheVolume(data: ComicVineVolume): void {
|
||||
if (!Number.isSafeInteger(data.id) || data.id <= 0) return;
|
||||
|
||||
if (this.volumeByIdCache.size >= VOLUME_CACHE_MAX_SIZE && !this.volumeByIdCache.has(data.id)) {
|
||||
const oldest = this.volumeByIdCache.keys().next().value;
|
||||
if (oldest !== undefined) this.volumeByIdCache.delete(oldest);
|
||||
}
|
||||
this.volumeByIdCache.set(data.id, { data, expiresAt: Date.now() + VOLUME_CACHE_TTL_MS });
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { MetadataProviderKey } from '@bookorbit/types';
|
||||
|
||||
import { mapIssueToCandidate } from './comicvine.mapper';
|
||||
import type { ComicVineIssue } from './comicvine.types';
|
||||
import type { ComicVineIssue, ComicVineVolume } from './comicvine.types';
|
||||
|
||||
function makeIssue(overrides: Partial<ComicVineIssue> = {}): ComicVineIssue {
|
||||
return {
|
||||
@@ -24,6 +24,21 @@ function makeIssue(overrides: Partial<ComicVineIssue> = {}): ComicVineIssue {
|
||||
};
|
||||
}
|
||||
|
||||
function makeVolume(overrides: Partial<ComicVineVolume> = {}): ComicVineVolume {
|
||||
return {
|
||||
id: 10,
|
||||
name: 'Series Name',
|
||||
start_year: '2024',
|
||||
count_of_issues: 50,
|
||||
description: null,
|
||||
deck: null,
|
||||
image: null,
|
||||
publisher: { id: 20, name: 'Example Comics' },
|
||||
site_detail_url: null,
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
describe('mapIssueToCandidate', () => {
|
||||
it('maps rich issue metadata including role-based contributor fields', () => {
|
||||
const candidate = mapIssueToCandidate(
|
||||
@@ -89,7 +104,7 @@ describe('mapIssueToCandidate', () => {
|
||||
|
||||
describe('series total books', () => {
|
||||
it('takes the issue count from the volume the caller searched', () => {
|
||||
const candidate = mapIssueToCandidate(makeIssue(), { volumeIssueCount: 50 });
|
||||
const candidate = mapIssueToCandidate(makeIssue(), { volume: makeVolume({ count_of_issues: 50 }) });
|
||||
expect(candidate.seriesTotalBooks).toBe(50);
|
||||
});
|
||||
|
||||
@@ -99,8 +114,35 @@ describe('mapIssueToCandidate', () => {
|
||||
});
|
||||
|
||||
it('rejects an out-of-range issue count', () => {
|
||||
expect(mapIssueToCandidate(makeIssue(), { volumeIssueCount: 0 }).seriesTotalBooks).toBeUndefined();
|
||||
expect(mapIssueToCandidate(makeIssue(), { volumeIssueCount: 10_001 }).seriesTotalBooks).toBeUndefined();
|
||||
expect(mapIssueToCandidate(makeIssue(), { volume: makeVolume({ count_of_issues: 0 }) }).seriesTotalBooks).toBeUndefined();
|
||||
expect(mapIssueToCandidate(makeIssue(), { volume: makeVolume({ count_of_issues: 10_001 }) }).seriesTotalBooks).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('publisher', () => {
|
||||
it('maps and trims the publisher from the linked volume', () => {
|
||||
const candidate = mapIssueToCandidate(makeIssue(), { volume: makeVolume({ publisher: { id: 20, name: ' DC Comics ' } }) });
|
||||
|
||||
expect(candidate.publisher).toBe('DC Comics');
|
||||
});
|
||||
|
||||
it('stays undefined when the linked volume has no usable publisher', () => {
|
||||
expect(mapIssueToCandidate(makeIssue(), { volume: makeVolume({ publisher: null }) }).publisher).toBeUndefined();
|
||||
expect(mapIssueToCandidate(makeIssue(), { volume: makeVolume({ publisher: { id: 20, name: ' ' } }) }).publisher).toBeUndefined();
|
||||
expect(
|
||||
mapIssueToCandidate(makeIssue(), {
|
||||
volume: makeVolume({ publisher: { id: 20, name: null as unknown as string } }),
|
||||
}).publisher,
|
||||
).toBeUndefined();
|
||||
});
|
||||
|
||||
it('rejects publisher and issue count from a volume belonging to another run', () => {
|
||||
const candidate = mapIssueToCandidate(makeIssue(), {
|
||||
volume: makeVolume({ id: 999, publisher: { id: 20, name: 'Wrong Publisher' }, count_of_issues: 99 }),
|
||||
});
|
||||
|
||||
expect(candidate.publisher).toBeUndefined();
|
||||
expect(candidate.seriesTotalBooks).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -2,7 +2,7 @@ import { ComicMetadataFields, MetadataCandidate, MetadataProviderKey } from '@bo
|
||||
|
||||
import { parsePublishedDateKey, parsePublishedYear, publishedYearFromDateKey } from '../../../../common/utils/published-date.utils';
|
||||
import { normalizeSeriesTotalBooks } from '../../../../common/utils/series-total-books.utils';
|
||||
import { ComicVineIssue, ComicVinePersonCredit } from './comicvine.types';
|
||||
import { ComicVineIssue, ComicVinePersonCredit, ComicVineVolume } from './comicvine.types';
|
||||
|
||||
function parseYear(dateStr: string | null | undefined): number | undefined {
|
||||
return parsePublishedYear(dateStr ?? undefined);
|
||||
@@ -45,15 +45,21 @@ function buildDisplayTitle(issue: ComicVineIssue): string {
|
||||
return parts.join(' ');
|
||||
}
|
||||
|
||||
function publisherName(volume: ComicVineVolume | undefined): string | undefined {
|
||||
const name: unknown = volume?.publisher?.name;
|
||||
return typeof name === 'string' ? name.trim() || undefined : undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* The issue payload only carries its volume's id and name, so the issue count has to come from
|
||||
* the volume record the caller already searched. Options object rather than a positional count so
|
||||
* that `.map(mapIssueToCandidate)` cannot silently pass the array index as the total.
|
||||
* The issue payload only carries its volume's id and name, so volume-level fields have to come
|
||||
* from the linked volume record. The id check prevents a mismatched search result from leaking
|
||||
* another run's publisher or issue count into the candidate.
|
||||
*/
|
||||
export function mapIssueToCandidate(issue: ComicVineIssue, options?: { volumeIssueCount?: number }): MetadataCandidate {
|
||||
export function mapIssueToCandidate(issue: ComicVineIssue, options?: { volume?: ComicVineVolume }): MetadataCandidate {
|
||||
const writers = extractByRole(issue.person_credits ?? [], 'writer');
|
||||
const rawPublishedDate = issue.cover_date ?? issue.store_date;
|
||||
const publishedDate = parsePublishedDateKey(rawPublishedDate ?? undefined);
|
||||
const volume = options?.volume?.id === issue.volume.id ? options.volume : undefined;
|
||||
|
||||
return {
|
||||
provider: MetadataProviderKey.COMICVINE,
|
||||
@@ -62,11 +68,12 @@ export function mapIssueToCandidate(issue: ComicVineIssue, options?: { volumeIss
|
||||
displayTitle: buildDisplayTitle(issue),
|
||||
authors: writers,
|
||||
description: issue.description ?? issue.deck ?? undefined,
|
||||
publisher: publisherName(volume),
|
||||
publishedDate,
|
||||
publishedYear: publishedDate ? publishedYearFromDateKey(publishedDate) : parseYear(rawPublishedDate),
|
||||
seriesName: issue.volume.name,
|
||||
seriesIndex: parseSeriesIndex(issue.issue_number),
|
||||
seriesTotalBooks: normalizeSeriesTotalBooks(options?.volumeIssueCount),
|
||||
seriesTotalBooks: normalizeSeriesTotalBooks(volume?.count_of_issues),
|
||||
coverUrl: issue.image?.original_url,
|
||||
sourceUrl: issue.site_detail_url ?? undefined,
|
||||
comicMetadata: buildComicMetadata(issue),
|
||||
|
||||
@@ -28,6 +28,7 @@ const mockVolume = {
|
||||
name: 'Batman',
|
||||
start_year: '2016',
|
||||
count_of_issues: 50,
|
||||
publisher: { id: 10, name: 'DC Comics' },
|
||||
};
|
||||
|
||||
const mockIssue = {
|
||||
@@ -59,6 +60,7 @@ describe('ComicVineProvider', () => {
|
||||
searchIssuesInVolume: vi.fn().mockResolvedValue([]),
|
||||
searchIssues: vi.fn().mockResolvedValue([]),
|
||||
getIssueById: vi.fn().mockResolvedValue(null),
|
||||
getVolumeById: vi.fn().mockResolvedValue(null),
|
||||
windowResetMs: vi.fn().mockReturnValue(0),
|
||||
},
|
||||
},
|
||||
@@ -137,6 +139,8 @@ describe('ComicVineProvider', () => {
|
||||
expect(client.searchVolumes).toHaveBeenCalledWith('Batman', 'test-key', expect.any(AbortSignal));
|
||||
expect(client.searchIssuesInVolume).toHaveBeenCalledWith(mockVolume.id, '1', 'test-key', expect.any(AbortSignal));
|
||||
expect(results).toHaveLength(1);
|
||||
expect(results[0]?.publisher).toBe('DC Comics');
|
||||
expect(client.getVolumeById).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('falls back to the stored series when the title is a bare issue name', async () => {
|
||||
@@ -301,6 +305,50 @@ describe('ComicVineProvider', () => {
|
||||
expect(client.searchVolumes).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('fetches the linked volume and maps its publisher', async () => {
|
||||
vi.mocked(client.searchIssues).mockResolvedValue([mockIssue]);
|
||||
vi.mocked(client.getVolumeById).mockResolvedValue(mockVolume);
|
||||
|
||||
const results = await provider.search({ title: 'Batman' });
|
||||
|
||||
expect(client.getVolumeById).toHaveBeenCalledWith(mockIssue.volume.id, 'test-key', expect.any(AbortSignal));
|
||||
expect(results[0]?.publisher).toBe('DC Comics');
|
||||
});
|
||||
|
||||
it('looks up a shared volume only once per search', async () => {
|
||||
const secondIssue = { ...mockIssue, id: 101, issue_number: '2' };
|
||||
vi.mocked(client.searchIssues).mockResolvedValue([mockIssue, secondIssue]);
|
||||
vi.mocked(client.getVolumeById).mockResolvedValue(mockVolume);
|
||||
|
||||
const results = await provider.search({ title: 'Batman' });
|
||||
|
||||
expect(client.getVolumeById).toHaveBeenCalledTimes(1);
|
||||
expect(results.map((candidate) => candidate.publisher)).toEqual(['DC Comics', 'DC Comics']);
|
||||
});
|
||||
|
||||
it('keeps the candidate when the linked volume cannot be loaded', async () => {
|
||||
vi.mocked(client.searchIssues).mockResolvedValue([mockIssue]);
|
||||
vi.mocked(client.getVolumeById).mockResolvedValue(null);
|
||||
|
||||
const results = await provider.search({ title: 'Batman' });
|
||||
|
||||
expect(results).toHaveLength(1);
|
||||
expect(results[0]?.publisher).toBeUndefined();
|
||||
});
|
||||
|
||||
it('keeps all candidates and stops enriching after a volume lookup is throttled', async () => {
|
||||
const secondIssue = { ...mockIssue, id: 101, volume: { id: 2, name: 'Detective Comics' } };
|
||||
vi.mocked(client.searchIssues).mockResolvedValue([mockIssue, secondIssue]);
|
||||
vi.mocked(client.getVolumeById).mockRejectedValueOnce(new ProviderThrottleError());
|
||||
vi.mocked(client.windowResetMs).mockReturnValue(61_000);
|
||||
|
||||
const results = await provider.search({ title: 'Batman' });
|
||||
|
||||
expect(results).toHaveLength(2);
|
||||
expect(client.getVolumeById).toHaveBeenCalledTimes(1);
|
||||
expect(throttleTracker.record).toHaveBeenCalledWith(MetadataProviderKey.COMICVINE, 61);
|
||||
});
|
||||
|
||||
it('returns empty array when searchIssues returns nothing', async () => {
|
||||
vi.spyOn(client, 'searchIssues').mockResolvedValue([]);
|
||||
|
||||
@@ -396,11 +444,13 @@ describe('ComicVineProvider', () => {
|
||||
|
||||
it('returns mapped candidate when issue is found', async () => {
|
||||
vi.spyOn(client, 'getIssueById').mockResolvedValue(mockIssue);
|
||||
vi.spyOn(client, 'getVolumeById').mockResolvedValue(mockVolume);
|
||||
|
||||
const result = await provider.lookupById('100');
|
||||
|
||||
expect(client.getIssueById).toHaveBeenCalledWith('100', 'test-key');
|
||||
expect(result).not.toBeNull();
|
||||
expect(client.getVolumeById).toHaveBeenCalledWith(mockIssue.volume.id, 'test-key');
|
||||
expect(result?.publisher).toBe('DC Comics');
|
||||
});
|
||||
|
||||
it('returns null when issue is not found', async () => {
|
||||
@@ -419,6 +469,37 @@ describe('ComicVineProvider', () => {
|
||||
expect(throttleTracker.record).toHaveBeenCalledWith(MetadataProviderKey.COMICVINE, 3600);
|
||||
});
|
||||
|
||||
it('keeps the issue candidate when volume enrichment is throttled', async () => {
|
||||
vi.mocked(client.getIssueById).mockResolvedValue(mockIssue);
|
||||
vi.mocked(client.getVolumeById).mockRejectedValue(new ProviderThrottleError());
|
||||
vi.mocked(client.windowResetMs).mockReturnValue(62_000);
|
||||
|
||||
const result = await provider.lookupById('100');
|
||||
|
||||
expect(result?.providerId).toBe('100');
|
||||
expect(result?.publisher).toBeUndefined();
|
||||
expect(throttleTracker.record).toHaveBeenCalledWith(MetadataProviderKey.COMICVINE, 62);
|
||||
});
|
||||
|
||||
it('keeps the issue candidate when the volume does not exist', async () => {
|
||||
vi.mocked(client.getIssueById).mockResolvedValue(mockIssue);
|
||||
vi.mocked(client.getVolumeById).mockResolvedValue(null);
|
||||
|
||||
const result = await provider.lookupById('100');
|
||||
|
||||
expect(result?.providerId).toBe('100');
|
||||
expect(result?.publisher).toBeUndefined();
|
||||
});
|
||||
|
||||
it('does not request a malformed volume id from an external payload', async () => {
|
||||
vi.mocked(client.getIssueById).mockResolvedValue({ ...mockIssue, volume: { id: Number.NaN, name: 'Batman' } });
|
||||
|
||||
const result = await provider.lookupById('100');
|
||||
|
||||
expect(result?.providerId).toBe('100');
|
||||
expect(client.getVolumeById).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('rethrows non-throttle errors', async () => {
|
||||
vi.spyOn(client, 'getIssueById').mockRejectedValue(new Error('DB error'));
|
||||
|
||||
|
||||
@@ -31,6 +31,11 @@ interface VolumeIssueMatch {
|
||||
volume?: ComicVineVolume;
|
||||
}
|
||||
|
||||
interface VolumeEnrichmentResult {
|
||||
matches: VolumeIssueMatch[];
|
||||
stopped: boolean;
|
||||
}
|
||||
|
||||
function parseIssueTitle(title: string): ParsedIssueTitle | null {
|
||||
const match = ISSUE_PATTERN.exec(title.trim());
|
||||
if (!match) return null;
|
||||
@@ -82,6 +87,10 @@ function hasAnyCredits(issue: ComicVineIssue): boolean {
|
||||
);
|
||||
}
|
||||
|
||||
function isValidVolumeId(volumeId: number): boolean {
|
||||
return Number.isSafeInteger(volumeId) && volumeId > 0;
|
||||
}
|
||||
|
||||
@Injectable()
|
||||
export class ComicVineProvider implements IdentifiableProvider {
|
||||
readonly key = MetadataProviderKey.COMICVINE;
|
||||
@@ -114,8 +123,11 @@ export class ComicVineProvider implements IdentifiableProvider {
|
||||
? await this.structuredSearch(parsed.seriesName, parsed.issueNumber, apiKey, maxCandidates, deadline)
|
||||
: await this.generalSearch(params.title, apiKey, maxCandidates, deadline);
|
||||
|
||||
const enriched = await this.enrichWithinBudget(matches, apiKey, deadline);
|
||||
return enriched.map(({ issue, volume }) => mapIssueToCandidate(issue, { volumeIssueCount: volume?.count_of_issues }));
|
||||
const volumeEnrichment = await this.enrichVolumesWithinBudget(matches, apiKey, deadline);
|
||||
const enriched = volumeEnrichment.stopped
|
||||
? volumeEnrichment.matches
|
||||
: await this.enrichIssueDetailsWithinBudget(volumeEnrichment.matches, apiKey, deadline);
|
||||
return enriched.map(({ issue, volume }) => mapIssueToCandidate(issue, { volume }));
|
||||
} finally {
|
||||
deadline.dispose();
|
||||
}
|
||||
@@ -134,7 +146,21 @@ export class ComicVineProvider implements IdentifiableProvider {
|
||||
|
||||
try {
|
||||
const issue = signal ? await this.client.getIssueById(providerId, apiKey, signal) : await this.client.getIssueById(providerId, apiKey);
|
||||
return issue ? mapIssueToCandidate(issue) : null;
|
||||
if (!issue) return null;
|
||||
if (!isValidVolumeId(issue.volume.id)) return mapIssueToCandidate(issue);
|
||||
|
||||
try {
|
||||
const volume = signal
|
||||
? await this.client.getVolumeById(issue.volume.id, apiKey, signal)
|
||||
: await this.client.getVolumeById(issue.volume.id, apiKey);
|
||||
return mapIssueToCandidate(issue, { volume: volume ?? undefined });
|
||||
} catch (err) {
|
||||
if (err instanceof ProviderThrottleError) {
|
||||
this.recordThrottle();
|
||||
return mapIssueToCandidate(issue);
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
} catch (err) {
|
||||
if (err instanceof ProviderThrottleError) {
|
||||
this.recordThrottle();
|
||||
@@ -213,13 +239,46 @@ export class ComicVineProvider implements IdentifiableProvider {
|
||||
return issues.slice(0, maxCandidates).map((issue) => ({ issue }));
|
||||
}
|
||||
|
||||
private async enrichVolumesWithinBudget(matches: VolumeIssueMatch[], apiKey: string, deadline: SearchDeadline): Promise<VolumeEnrichmentResult> {
|
||||
const enriched: VolumeIssueMatch[] = [];
|
||||
const volumes = new Map<number, ComicVineVolume | null>();
|
||||
let stopped = false;
|
||||
|
||||
for (const match of matches) {
|
||||
const volumeId = match.issue.volume.id;
|
||||
if (stopped || match.volume || !isValidVolumeId(volumeId) || deadline.expired()) {
|
||||
enriched.push(match);
|
||||
continue;
|
||||
}
|
||||
|
||||
if (volumes.has(volumeId)) {
|
||||
enriched.push({ ...match, volume: volumes.get(volumeId) ?? undefined });
|
||||
continue;
|
||||
}
|
||||
|
||||
try {
|
||||
const volume = await this.client.getVolumeById(volumeId, apiKey, deadline.signal);
|
||||
const linkedVolume = volume?.id === volumeId ? volume : null;
|
||||
volumes.set(volumeId, linkedVolume);
|
||||
enriched.push({ ...match, volume: linkedVolume ?? undefined });
|
||||
} catch (err) {
|
||||
enriched.push(match);
|
||||
stopped = true;
|
||||
if (err instanceof ProviderThrottleError) this.recordThrottle();
|
||||
}
|
||||
}
|
||||
|
||||
return { matches: enriched, stopped };
|
||||
}
|
||||
|
||||
/**
|
||||
* Detail lookups only add credits, so they are worth a request each only while the budget holds.
|
||||
* Detail lookups only add credits, so they are worth a request each only after volume fields have
|
||||
* been filled and while the budget holds.
|
||||
* Sequential because the client serializes calls anyway, which lets the deadline actually stop
|
||||
* the work rather than every lookup being queued up front. A lookup that fails or is cut off
|
||||
* costs its candidate only the credits, never the candidate itself.
|
||||
*/
|
||||
private async enrichWithinBudget(matches: VolumeIssueMatch[], apiKey: string, deadline: SearchDeadline): Promise<VolumeIssueMatch[]> {
|
||||
private async enrichIssueDetailsWithinBudget(matches: VolumeIssueMatch[], apiKey: string, deadline: SearchDeadline): Promise<VolumeIssueMatch[]> {
|
||||
const enriched: VolumeIssueMatch[] = [];
|
||||
let stopped = false;
|
||||
|
||||
|
||||
+15
-1
@@ -124,6 +124,7 @@ interface HarnessOptions {
|
||||
// Issues the /issues/ endpoint holds, keyed "<volumeId>#<issueNumber>".
|
||||
issues?: Record<string, ComicVineIssue[]>;
|
||||
generalSearchResults?: ComicVineIssue[];
|
||||
volumeDetails?: Record<number, ComicVineVolume>;
|
||||
issueDetails?: Record<string, ComicVineIssue>;
|
||||
issueDetailMs?: number;
|
||||
// Volumes and issue details ComicVine answers with HTTP 420, its throttle response.
|
||||
@@ -137,6 +138,7 @@ interface Harness {
|
||||
throttleTracker: ProviderThrottleTracker;
|
||||
requestUrls: URL[];
|
||||
probedVolumeIds: number[];
|
||||
detailVolumeIds: number[];
|
||||
detailIssueIds: string[];
|
||||
abortedPaths: string[];
|
||||
}
|
||||
@@ -146,6 +148,7 @@ function buildHarness(options: HarnessOptions = {}): Harness {
|
||||
const issues = options.issues ?? {};
|
||||
const requestUrls: URL[] = [];
|
||||
const probedVolumeIds: number[] = [];
|
||||
const detailVolumeIds: number[] = [];
|
||||
const detailIssueIds: string[] = [];
|
||||
const abortedPaths: string[] = [];
|
||||
const detailMs = options.issueDetailMs ?? ISSUE_DETAIL_MS;
|
||||
@@ -183,6 +186,12 @@ function buildHarness(options: HarnessOptions = {}): Harness {
|
||||
);
|
||||
}
|
||||
|
||||
const volumeId = Number(/\/volume\/4050-(\d+)\//.exec(target.pathname)?.[1]);
|
||||
if (Number.isSafeInteger(volumeId)) {
|
||||
detailVolumeIds.push(volumeId);
|
||||
return respondAfter(detailMs, () => okResponse(options.volumeDetails?.[volumeId] ?? null));
|
||||
}
|
||||
|
||||
const issueId = /\/issue\/4000-(\d+)\//.exec(target.pathname)?.[1] ?? '';
|
||||
detailIssueIds.push(issueId);
|
||||
return respondAfter(detailMs, () =>
|
||||
@@ -199,7 +208,7 @@ function buildHarness(options: HarnessOptions = {}): Harness {
|
||||
);
|
||||
const service = new MetadataFetchService(new ProviderRegistry([provider]), new ProviderThrottleTracker(), {} as MetadataFetchRepository);
|
||||
|
||||
return { provider, service, throttleTracker, requestUrls, probedVolumeIds, detailIssueIds, abortedPaths };
|
||||
return { provider, service, throttleTracker, requestUrls, probedVolumeIds, detailVolumeIds, detailIssueIds, abortedPaths };
|
||||
}
|
||||
|
||||
async function runToCompletion<T>(work: Promise<T>): Promise<{ value: T; elapsedMs: number }> {
|
||||
@@ -399,12 +408,17 @@ describe('ComicVine search budget', () => {
|
||||
it('falls back to the general issue search for a comic with no parsed series', async () => {
|
||||
const harness = buildHarness({
|
||||
generalSearchResults: [issue(900_001, ASM_2022, 'The Amazing Spider-Man', '67')],
|
||||
volumeDetails: {
|
||||
[ASM_2022]: { ...volume(ASM_2022, 'The Amazing Spider-Man', '2022', 93), publisher: { id: 31, name: 'Marvel' } },
|
||||
},
|
||||
});
|
||||
|
||||
const { value: candidates } = await runToCompletion(harness.provider.search({ title: 'The Amazing Spider-Man (2022) Volume 06 Issue 067' }));
|
||||
|
||||
expect(harness.requestUrls[0].pathname).toBe('/api/search/');
|
||||
expect(harness.probedVolumeIds).toEqual([]);
|
||||
expect(harness.detailVolumeIds).toEqual([ASM_2022]);
|
||||
expect(candidates).toHaveLength(1);
|
||||
expect(candidates[0]?.publisher).toBe('Marvel');
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user