mirror of
https://github.com/bookorbit/bookorbit.git
synced 2026-11-03 08:21:39 -06:00
feat: reactive ComicVine throttling with precise window-based cooldown
- replace fixed RateLimiter with passive timestamp tracker exposing windowResetMs() - record throttle in ProviderThrottleTracker on 420 with exact time until hourly window resets - inject ProviderThrottleTracker into ComicVineProvider; catch ProviderThrottleError in search and lookupById - add unit tests for comicvine.client and comicvine.provider (49 tests)
This commit is contained in:
@@ -0,0 +1,348 @@
|
|||||||
|
import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest';
|
||||||
|
|
||||||
|
import * as fetchWithThrottleModule from '../../fetch-with-throttle';
|
||||||
|
import { ProviderThrottleError } from '../../provider-throttle.error';
|
||||||
|
import { ComicVineClient } from './comicvine.client';
|
||||||
|
|
||||||
|
vi.mock('../../fetch-with-throttle', () => ({
|
||||||
|
fetchWithThrottle: vi.fn(),
|
||||||
|
}));
|
||||||
|
|
||||||
|
const HOUR_MS = 3_600_000;
|
||||||
|
|
||||||
|
function makeOkResponse(results: unknown, isArray = true): Response {
|
||||||
|
return {
|
||||||
|
ok: true,
|
||||||
|
status: 200,
|
||||||
|
json: () =>
|
||||||
|
Promise.resolve({
|
||||||
|
status_code: 1,
|
||||||
|
error: 'OK',
|
||||||
|
results: isArray ? results : results,
|
||||||
|
}),
|
||||||
|
} as Response;
|
||||||
|
}
|
||||||
|
|
||||||
|
function makeErrorResponse(status: number): Response {
|
||||||
|
return { ok: false, status } as Response;
|
||||||
|
}
|
||||||
|
|
||||||
|
function make420Response(): Response {
|
||||||
|
return { ok: false, status: 420 } as Response;
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('ComicVineClient', () => {
|
||||||
|
let client: ComicVineClient;
|
||||||
|
let mockFetch: ReturnType<typeof vi.fn>;
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
vi.useFakeTimers();
|
||||||
|
vi.setSystemTime(new Date('2026-04-11T10:00:00.000Z'));
|
||||||
|
client = new ComicVineClient();
|
||||||
|
mockFetch = vi.mocked(fetchWithThrottleModule.fetchWithThrottle);
|
||||||
|
mockFetch.mockClear();
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
vi.useRealTimers();
|
||||||
|
vi.restoreAllMocks();
|
||||||
|
});
|
||||||
|
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
// RateLimiter / velocity guard
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
describe('velocity guard', () => {
|
||||||
|
it('allows the first request immediately without delay', async () => {
|
||||||
|
mockFetch.mockResolvedValue(makeOkResponse([]));
|
||||||
|
const before = Date.now();
|
||||||
|
const p = client.searchVolumes('Batman', 'key');
|
||||||
|
await vi.runAllTimersAsync();
|
||||||
|
await p;
|
||||||
|
expect(Date.now() - before).toBe(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('spaces consecutive requests 1 second apart', async () => {
|
||||||
|
mockFetch.mockResolvedValue(makeOkResponse([]));
|
||||||
|
|
||||||
|
// Start and fully settle the first request so nextAllowedTime is set
|
||||||
|
const p1 = client.searchVolumes('Batman', 'key');
|
||||||
|
await vi.runAllTimersAsync();
|
||||||
|
await p1;
|
||||||
|
expect(mockFetch).toHaveBeenCalledTimes(1);
|
||||||
|
|
||||||
|
// Second request should wait 1 second before firing
|
||||||
|
const p2 = client.searchIssues('Batman', 'key');
|
||||||
|
await vi.advanceTimersByTimeAsync(999);
|
||||||
|
expect(mockFetch).toHaveBeenCalledTimes(1);
|
||||||
|
|
||||||
|
await vi.advanceTimersByTimeAsync(1);
|
||||||
|
expect(mockFetch).toHaveBeenCalledTimes(2);
|
||||||
|
|
||||||
|
await p2;
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
// windowResetMs
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
describe('windowResetMs', () => {
|
||||||
|
it('returns 0 when no requests have been made', () => {
|
||||||
|
expect(client.windowResetMs()).toBe(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns time until oldest request exits the 1-hour window', async () => {
|
||||||
|
mockFetch.mockResolvedValue(makeOkResponse([]));
|
||||||
|
const p = client.searchVolumes('Batman', 'key');
|
||||||
|
await vi.runAllTimersAsync();
|
||||||
|
await p;
|
||||||
|
|
||||||
|
// Oldest request was at t=0; window resets at t+1h
|
||||||
|
expect(client.windowResetMs()).toBe(HOUR_MS);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('decreases as time passes', async () => {
|
||||||
|
mockFetch.mockResolvedValue(makeOkResponse([]));
|
||||||
|
const p = client.searchVolumes('Batman', 'key');
|
||||||
|
await vi.runAllTimersAsync();
|
||||||
|
await p;
|
||||||
|
|
||||||
|
vi.advanceTimersByTime(30 * 60 * 1000); // 30 minutes pass
|
||||||
|
expect(client.windowResetMs()).toBe(30 * 60 * 1000);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns 0 after the window has fully elapsed', async () => {
|
||||||
|
mockFetch.mockResolvedValue(makeOkResponse([]));
|
||||||
|
const p = client.searchVolumes('Batman', 'key');
|
||||||
|
await vi.runAllTimersAsync();
|
||||||
|
await p;
|
||||||
|
|
||||||
|
vi.advanceTimersByTime(HOUR_MS + 1);
|
||||||
|
expect(client.windowResetMs()).toBe(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('reflects the oldest in-window timestamp when multiple requests exist', async () => {
|
||||||
|
mockFetch.mockResolvedValue(makeOkResponse([]));
|
||||||
|
|
||||||
|
// Two requests 10 minutes apart
|
||||||
|
const p1 = client.searchVolumes('Batman', 'key');
|
||||||
|
await vi.runAllTimersAsync();
|
||||||
|
await p1;
|
||||||
|
|
||||||
|
vi.advanceTimersByTime(10 * 60 * 1000);
|
||||||
|
|
||||||
|
const p2 = client.searchIssues('Batman', 'key');
|
||||||
|
await vi.runAllTimersAsync();
|
||||||
|
await p2;
|
||||||
|
|
||||||
|
// Oldest is the first one — window resets 1h after that
|
||||||
|
expect(client.windowResetMs()).toBe(50 * 60 * 1000);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('evicts expired timestamps and returns 0 when all have exited the window', async () => {
|
||||||
|
mockFetch.mockResolvedValue(makeOkResponse([]));
|
||||||
|
|
||||||
|
const p = client.searchVolumes('Batman', 'key');
|
||||||
|
await vi.runAllTimersAsync();
|
||||||
|
await p;
|
||||||
|
|
||||||
|
vi.advanceTimersByTime(HOUR_MS + 1000);
|
||||||
|
expect(client.windowResetMs()).toBe(0);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
// searchVolumes
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
describe('searchVolumes', () => {
|
||||||
|
it('returns mapped volumes on success', async () => {
|
||||||
|
const volumes = [{ id: 1, name: 'Batman' }];
|
||||||
|
mockFetch.mockResolvedValue(makeOkResponse(volumes));
|
||||||
|
|
||||||
|
const p = client.searchVolumes('Batman', 'key');
|
||||||
|
await vi.runAllTimersAsync();
|
||||||
|
const result = await p;
|
||||||
|
|
||||||
|
expect(result).toEqual(volumes);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns cached result without making a new request', async () => {
|
||||||
|
const volumes = [{ id: 1, name: 'Batman' }];
|
||||||
|
mockFetch.mockResolvedValue(makeOkResponse(volumes));
|
||||||
|
|
||||||
|
const p1 = client.searchVolumes('batman', 'key');
|
||||||
|
await vi.runAllTimersAsync();
|
||||||
|
await p1;
|
||||||
|
|
||||||
|
const callCount = mockFetch.mock.calls.length;
|
||||||
|
|
||||||
|
const p2 = client.searchVolumes('Batman', 'key'); // different casing
|
||||||
|
await vi.runAllTimersAsync();
|
||||||
|
const result = await p2;
|
||||||
|
|
||||||
|
expect(mockFetch).toHaveBeenCalledTimes(callCount); // no new request
|
||||||
|
expect(result).toEqual(volumes);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('bypasses cache after TTL expires', async () => {
|
||||||
|
mockFetch.mockResolvedValue(makeOkResponse([{ id: 1, name: 'Batman' }]));
|
||||||
|
|
||||||
|
const p1 = client.searchVolumes('Batman', 'key');
|
||||||
|
await vi.runAllTimersAsync();
|
||||||
|
await p1;
|
||||||
|
|
||||||
|
vi.advanceTimersByTime(10 * 60 * 1000 + 1); // past 10-min TTL
|
||||||
|
|
||||||
|
mockFetch.mockResolvedValue(makeOkResponse([{ id: 2, name: 'Batman Vol 2' }]));
|
||||||
|
const p2 = client.searchVolumes('Batman', 'key');
|
||||||
|
await vi.runAllTimersAsync();
|
||||||
|
const result = await p2;
|
||||||
|
|
||||||
|
expect(result).toEqual([{ id: 2, name: 'Batman Vol 2' }]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns empty array on non-ok response', async () => {
|
||||||
|
mockFetch.mockResolvedValue(makeErrorResponse(500));
|
||||||
|
|
||||||
|
const p = client.searchVolumes('Batman', 'key');
|
||||||
|
await vi.runAllTimersAsync();
|
||||||
|
|
||||||
|
expect(await p).toEqual([]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns empty array when API status_code is not 1', async () => {
|
||||||
|
mockFetch.mockResolvedValue({
|
||||||
|
ok: true,
|
||||||
|
status: 200,
|
||||||
|
json: () => Promise.resolve({ status_code: 100, error: 'Invalid API Key', results: null }),
|
||||||
|
} as Response);
|
||||||
|
|
||||||
|
const p = client.searchVolumes('Batman', 'key');
|
||||||
|
await vi.runAllTimersAsync();
|
||||||
|
|
||||||
|
expect(await p).toEqual([]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('throws ProviderThrottleError on 420', async () => {
|
||||||
|
mockFetch.mockResolvedValue(make420Response());
|
||||||
|
|
||||||
|
const p = client.searchVolumes('Batman', 'key');
|
||||||
|
const assertion = expect(p).rejects.toThrow(ProviderThrottleError);
|
||||||
|
await vi.runAllTimersAsync();
|
||||||
|
await assertion;
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns empty array on network error', async () => {
|
||||||
|
mockFetch.mockRejectedValue(new Error('Network error'));
|
||||||
|
|
||||||
|
const p = client.searchVolumes('Batman', 'key');
|
||||||
|
await vi.runAllTimersAsync();
|
||||||
|
|
||||||
|
expect(await p).toEqual([]);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
// searchIssuesInVolume
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
describe('searchIssuesInVolume', () => {
|
||||||
|
it('returns issues on success', async () => {
|
||||||
|
const issues = [{ id: 10, issue_number: '1' }];
|
||||||
|
mockFetch.mockResolvedValue(makeOkResponse(issues));
|
||||||
|
|
||||||
|
const p = client.searchIssuesInVolume(1, '1', 'key');
|
||||||
|
await vi.runAllTimersAsync();
|
||||||
|
|
||||||
|
expect(await p).toEqual(issues);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns empty array on non-ok response', async () => {
|
||||||
|
mockFetch.mockResolvedValue(makeErrorResponse(503));
|
||||||
|
|
||||||
|
const p = client.searchIssuesInVolume(1, '1', 'key');
|
||||||
|
await vi.runAllTimersAsync();
|
||||||
|
|
||||||
|
expect(await p).toEqual([]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('throws ProviderThrottleError on 420', async () => {
|
||||||
|
mockFetch.mockResolvedValue(make420Response());
|
||||||
|
|
||||||
|
const p = client.searchIssuesInVolume(1, '1', 'key');
|
||||||
|
const assertion = expect(p).rejects.toThrow(ProviderThrottleError);
|
||||||
|
await vi.runAllTimersAsync();
|
||||||
|
await assertion;
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
// searchIssues
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
describe('searchIssues', () => {
|
||||||
|
it('returns issues on success', async () => {
|
||||||
|
const issues = [{ id: 20, name: 'Batman #1' }];
|
||||||
|
mockFetch.mockResolvedValue(makeOkResponse(issues));
|
||||||
|
|
||||||
|
const p = client.searchIssues('Batman #1', 'key');
|
||||||
|
await vi.runAllTimersAsync();
|
||||||
|
|
||||||
|
expect(await p).toEqual(issues);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('throws ProviderThrottleError on 420', async () => {
|
||||||
|
mockFetch.mockResolvedValue(make420Response());
|
||||||
|
|
||||||
|
const p = client.searchIssues('Batman', 'key');
|
||||||
|
const assertion = expect(p).rejects.toThrow(ProviderThrottleError);
|
||||||
|
await vi.runAllTimersAsync();
|
||||||
|
await assertion;
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
// getIssueById
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
describe('getIssueById', () => {
|
||||||
|
it('returns the issue on success', async () => {
|
||||||
|
const issue = { id: 999, name: 'Batman #1' };
|
||||||
|
mockFetch.mockResolvedValue(makeOkResponse(issue, false));
|
||||||
|
|
||||||
|
const p = client.getIssueById('999', 'key');
|
||||||
|
await vi.runAllTimersAsync();
|
||||||
|
|
||||||
|
expect(await p).toEqual(issue);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns null on non-ok response', async () => {
|
||||||
|
mockFetch.mockResolvedValue(makeErrorResponse(404));
|
||||||
|
|
||||||
|
const p = client.getIssueById('999', 'key');
|
||||||
|
await vi.runAllTimersAsync();
|
||||||
|
|
||||||
|
expect(await p).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('throws ProviderThrottleError on 420', async () => {
|
||||||
|
mockFetch.mockResolvedValue(make420Response());
|
||||||
|
|
||||||
|
const p = client.getIssueById('999', 'key');
|
||||||
|
const assertion = expect(p).rejects.toThrow(ProviderThrottleError);
|
||||||
|
await vi.runAllTimersAsync();
|
||||||
|
await assertion;
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns null on network error', async () => {
|
||||||
|
mockFetch.mockRejectedValue(new Error('timeout'));
|
||||||
|
|
||||||
|
const p = client.getIssueById('999', 'key');
|
||||||
|
await vi.runAllTimersAsync();
|
||||||
|
|
||||||
|
expect(await p).toBeNull();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -6,7 +6,8 @@ import { ComicVineApiResponse, ComicVineIssue, ComicVineVolume } from './comicvi
|
|||||||
|
|
||||||
const BASE_URL = 'https://comicvine.gamespot.com/api';
|
const BASE_URL = 'https://comicvine.gamespot.com/api';
|
||||||
const REQUEST_TIMEOUT_MS = 15_000;
|
const REQUEST_TIMEOUT_MS = 15_000;
|
||||||
const RATE_LIMIT_DELAY_MS = 1_000;
|
const VELOCITY_GUARD_MS = 1_000;
|
||||||
|
const HOURLY_WINDOW_MS = 3_600_000;
|
||||||
const VOLUME_CACHE_TTL_MS = 10 * 60 * 1_000;
|
const VOLUME_CACHE_TTL_MS = 10 * 60 * 1_000;
|
||||||
const VOLUME_CACHE_MAX_SIZE = 50;
|
const VOLUME_CACHE_MAX_SIZE = 50;
|
||||||
|
|
||||||
@@ -25,16 +26,28 @@ interface CachedVolumes {
|
|||||||
|
|
||||||
class RateLimiter {
|
class RateLimiter {
|
||||||
private nextAllowedTime = 0;
|
private nextAllowedTime = 0;
|
||||||
|
private readonly timestamps: number[] = [];
|
||||||
|
|
||||||
async throttle(): Promise<void> {
|
async throttle(): Promise<void> {
|
||||||
const now = Date.now();
|
const now = Date.now();
|
||||||
const scheduled = Math.max(now, this.nextAllowedTime);
|
const scheduled = Math.max(now, this.nextAllowedTime);
|
||||||
this.nextAllowedTime = scheduled + RATE_LIMIT_DELAY_MS;
|
this.nextAllowedTime = scheduled + VELOCITY_GUARD_MS;
|
||||||
|
this.timestamps.push(scheduled);
|
||||||
const wait = scheduled - now;
|
const wait = scheduled - now;
|
||||||
if (wait > 0) {
|
if (wait > 0) {
|
||||||
await new Promise<void>((resolve) => setTimeout(resolve, wait));
|
await new Promise<void>((resolve) => setTimeout(resolve, wait));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
timeUntilWindowResetMs(): number {
|
||||||
|
const now = Date.now();
|
||||||
|
const windowStart = now - HOURLY_WINDOW_MS;
|
||||||
|
while (this.timestamps.length > 0 && this.timestamps[0] < windowStart) {
|
||||||
|
this.timestamps.shift();
|
||||||
|
}
|
||||||
|
if (this.timestamps.length === 0) return 0;
|
||||||
|
return Math.max(0, this.timestamps[0] + HOURLY_WINDOW_MS - now);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@Injectable()
|
@Injectable()
|
||||||
@@ -43,6 +56,10 @@ export class ComicVineClient {
|
|||||||
private readonly rateLimiter = new RateLimiter();
|
private readonly rateLimiter = new RateLimiter();
|
||||||
private readonly volumeCache = new Map<string, CachedVolumes>();
|
private readonly volumeCache = new Map<string, CachedVolumes>();
|
||||||
|
|
||||||
|
windowResetMs(): number {
|
||||||
|
return this.rateLimiter.timeUntilWindowResetMs();
|
||||||
|
}
|
||||||
|
|
||||||
async searchVolumes(seriesName: string, apiKey: string): Promise<ComicVineVolume[]> {
|
async searchVolumes(seriesName: string, apiKey: string): Promise<ComicVineVolume[]> {
|
||||||
const cacheKey = seriesName.toLowerCase().trim();
|
const cacheKey = seriesName.toLowerCase().trim();
|
||||||
const cached = this.volumeCache.get(cacheKey);
|
const cached = this.volumeCache.get(cacheKey);
|
||||||
|
|||||||
@@ -0,0 +1,360 @@
|
|||||||
|
import { Test, TestingModule } from '@nestjs/testing';
|
||||||
|
import { MetadataProviderKey, ProviderConfigurations } from '@projectx/types';
|
||||||
|
import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest';
|
||||||
|
|
||||||
|
import { ProviderConfigService } from '../../../metadata-preferences/provider-config.service';
|
||||||
|
import { ProviderThrottleError } from '../../provider-throttle.error';
|
||||||
|
import { ProviderThrottleTracker } from '../../provider-throttle.tracker';
|
||||||
|
import { ComicVineClient } from './comicvine.client';
|
||||||
|
import { ComicVineProvider } from './comicvine.provider';
|
||||||
|
|
||||||
|
const mockConfig: ProviderConfigurations = {
|
||||||
|
google: { enabled: false, apiKey: '' },
|
||||||
|
amazon: { enabled: false, domain: 'amazon.com', cookie: '' },
|
||||||
|
goodreads: { enabled: false },
|
||||||
|
hardcover: { enabled: false, apiKey: '' },
|
||||||
|
openLibrary: { enabled: false },
|
||||||
|
itunes: { enabled: false, coverResolution: 'high' },
|
||||||
|
audible: { enabled: false, domain: 'com' },
|
||||||
|
audnexus: { enabled: false },
|
||||||
|
comicvine: { enabled: true, apiKey: 'test-key' },
|
||||||
|
};
|
||||||
|
|
||||||
|
const mockVolume = {
|
||||||
|
id: 1,
|
||||||
|
name: 'Batman',
|
||||||
|
start_year: '2016',
|
||||||
|
count_of_issues: 50,
|
||||||
|
};
|
||||||
|
|
||||||
|
const mockIssue = {
|
||||||
|
id: 100,
|
||||||
|
name: 'Batman #1',
|
||||||
|
issue_number: '1',
|
||||||
|
volume: { id: 1, name: 'Batman' },
|
||||||
|
person_credits: [{ id: 1, name: 'Tom King', role: 'writer' }],
|
||||||
|
character_credits: [],
|
||||||
|
team_credits: [],
|
||||||
|
story_arc_credits: [],
|
||||||
|
location_credits: [],
|
||||||
|
};
|
||||||
|
|
||||||
|
describe('ComicVineProvider', () => {
|
||||||
|
let provider: ComicVineProvider;
|
||||||
|
let client: ComicVineClient;
|
||||||
|
let providerConfig: ProviderConfigService;
|
||||||
|
let throttleTracker: ProviderThrottleTracker;
|
||||||
|
|
||||||
|
beforeEach(async () => {
|
||||||
|
const module: TestingModule = await Test.createTestingModule({
|
||||||
|
providers: [
|
||||||
|
ComicVineProvider,
|
||||||
|
{
|
||||||
|
provide: ComicVineClient,
|
||||||
|
useValue: {
|
||||||
|
searchVolumes: vi.fn().mockResolvedValue([]),
|
||||||
|
searchIssuesInVolume: vi.fn().mockResolvedValue([]),
|
||||||
|
searchIssues: vi.fn().mockResolvedValue([]),
|
||||||
|
getIssueById: vi.fn().mockResolvedValue(null),
|
||||||
|
windowResetMs: vi.fn().mockReturnValue(0),
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
provide: ProviderConfigService,
|
||||||
|
useValue: {
|
||||||
|
getConfig: vi.fn().mockResolvedValue(mockConfig),
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
provide: ProviderThrottleTracker,
|
||||||
|
useValue: {
|
||||||
|
record: vi.fn(),
|
||||||
|
isThrottled: vi.fn().mockReturnValue(false),
|
||||||
|
clearOnSuccess: vi.fn(),
|
||||||
|
},
|
||||||
|
},
|
||||||
|
],
|
||||||
|
}).compile();
|
||||||
|
|
||||||
|
provider = module.get(ComicVineProvider);
|
||||||
|
client = module.get(ComicVineClient);
|
||||||
|
providerConfig = module.get(ProviderConfigService);
|
||||||
|
throttleTracker = module.get(ProviderThrottleTracker);
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
vi.restoreAllMocks();
|
||||||
|
});
|
||||||
|
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
// search — guard clauses
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
describe('search - disabled / missing config', () => {
|
||||||
|
it('returns empty array when provider is disabled', async () => {
|
||||||
|
vi.spyOn(providerConfig, 'getConfig').mockResolvedValue({
|
||||||
|
...mockConfig,
|
||||||
|
comicvine: { enabled: false, apiKey: 'test-key' },
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(await provider.search({ title: 'Batman #1' })).toEqual([]);
|
||||||
|
expect(client.searchVolumes).not.toHaveBeenCalled();
|
||||||
|
expect(client.searchIssues).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns empty array when apiKey is missing', async () => {
|
||||||
|
vi.spyOn(providerConfig, 'getConfig').mockResolvedValue({
|
||||||
|
...mockConfig,
|
||||||
|
comicvine: { enabled: true, apiKey: '' },
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(await provider.search({ title: 'Batman #1' })).toEqual([]);
|
||||||
|
expect(client.searchVolumes).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns empty array when title is missing', async () => {
|
||||||
|
expect(await provider.search({})).toEqual([]);
|
||||||
|
expect(client.searchVolumes).not.toHaveBeenCalled();
|
||||||
|
expect(client.searchIssues).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
// search — structured path (title matches "Series #N" pattern)
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
describe('search - structured path', () => {
|
||||||
|
it('calls searchVolumes then searchIssuesInVolume for a parsed title', async () => {
|
||||||
|
vi.spyOn(client, 'searchVolumes').mockResolvedValue([mockVolume]);
|
||||||
|
vi.spyOn(client, 'searchIssuesInVolume').mockResolvedValue([mockIssue]);
|
||||||
|
|
||||||
|
const results = await provider.search({ title: 'Batman #1' });
|
||||||
|
|
||||||
|
expect(client.searchVolumes).toHaveBeenCalledWith('Batman', 'test-key');
|
||||||
|
expect(client.searchIssuesInVolume).toHaveBeenCalledWith(mockVolume.id, '1', 'test-key');
|
||||||
|
expect(results).toHaveLength(1);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('tries volumes sorted by start_year descending', async () => {
|
||||||
|
const older = { ...mockVolume, id: 1, start_year: '1990' };
|
||||||
|
const newer = { ...mockVolume, id: 2, start_year: '2016' };
|
||||||
|
vi.spyOn(client, 'searchVolumes').mockResolvedValue([older, newer]);
|
||||||
|
vi.spyOn(client, 'searchIssuesInVolume').mockResolvedValue([mockIssue]);
|
||||||
|
|
||||||
|
await provider.search({ title: 'Batman #1' });
|
||||||
|
|
||||||
|
expect(client.searchIssuesInVolume).toHaveBeenNthCalledWith(1, newer.id, '1', 'test-key');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns empty array when no volumes are found', async () => {
|
||||||
|
vi.spyOn(client, 'searchVolumes').mockResolvedValue([]);
|
||||||
|
|
||||||
|
expect(await provider.search({ title: 'Batman #1' })).toEqual([]);
|
||||||
|
expect(client.searchIssuesInVolume).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('tries next volume when first has no matching issues', async () => {
|
||||||
|
const v1 = { ...mockVolume, id: 1 };
|
||||||
|
const v2 = { ...mockVolume, id: 2 };
|
||||||
|
vi.spyOn(client, 'searchVolumes').mockResolvedValue([v1, v2]);
|
||||||
|
vi.spyOn(client, 'searchIssuesInVolume').mockResolvedValueOnce([]).mockResolvedValueOnce([mockIssue]);
|
||||||
|
|
||||||
|
const results = await provider.search({ title: 'Batman #1' });
|
||||||
|
|
||||||
|
expect(client.searchIssuesInVolume).toHaveBeenCalledTimes(2);
|
||||||
|
expect(results).toHaveLength(1);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns empty array when no volume has matching issues', async () => {
|
||||||
|
vi.spyOn(client, 'searchVolumes').mockResolvedValue([mockVolume]);
|
||||||
|
vi.spyOn(client, 'searchIssuesInVolume').mockResolvedValue([]);
|
||||||
|
|
||||||
|
expect(await provider.search({ title: 'Batman #1' })).toEqual([]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('fetches issue detail when issue has no credits', async () => {
|
||||||
|
const issueNoCredits = {
|
||||||
|
...mockIssue,
|
||||||
|
person_credits: [],
|
||||||
|
character_credits: [],
|
||||||
|
team_credits: [],
|
||||||
|
story_arc_credits: [],
|
||||||
|
location_credits: [],
|
||||||
|
};
|
||||||
|
const issueWithCredits = { ...mockIssue };
|
||||||
|
vi.spyOn(client, 'searchVolumes').mockResolvedValue([mockVolume]);
|
||||||
|
vi.spyOn(client, 'searchIssuesInVolume').mockResolvedValue([issueNoCredits]);
|
||||||
|
vi.spyOn(client, 'getIssueById').mockResolvedValue(issueWithCredits);
|
||||||
|
|
||||||
|
await provider.search({ title: 'Batman #1' });
|
||||||
|
|
||||||
|
expect(client.getIssueById).toHaveBeenCalledWith(String(issueNoCredits.id), 'test-key');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('skips detail fetch when issue already has credits', async () => {
|
||||||
|
vi.spyOn(client, 'searchVolumes').mockResolvedValue([mockVolume]);
|
||||||
|
vi.spyOn(client, 'searchIssuesInVolume').mockResolvedValue([mockIssue]);
|
||||||
|
|
||||||
|
await provider.search({ title: 'Batman #1' });
|
||||||
|
|
||||||
|
expect(client.getIssueById).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('falls back to list-endpoint issue if detail fetch returns null', async () => {
|
||||||
|
const issueNoCredits = {
|
||||||
|
...mockIssue,
|
||||||
|
person_credits: [],
|
||||||
|
character_credits: [],
|
||||||
|
team_credits: [],
|
||||||
|
story_arc_credits: [],
|
||||||
|
location_credits: [],
|
||||||
|
};
|
||||||
|
vi.spyOn(client, 'searchVolumes').mockResolvedValue([mockVolume]);
|
||||||
|
vi.spyOn(client, 'searchIssuesInVolume').mockResolvedValue([issueNoCredits]);
|
||||||
|
vi.spyOn(client, 'getIssueById').mockResolvedValue(null);
|
||||||
|
|
||||||
|
const results = await provider.search({ title: 'Batman #1' });
|
||||||
|
|
||||||
|
expect(results).toHaveLength(1);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
// search — general path (title does not match pattern)
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
describe('search - general path', () => {
|
||||||
|
it('calls searchIssues when title does not match the issue pattern', async () => {
|
||||||
|
vi.spyOn(client, 'searchIssues').mockResolvedValue([mockIssue]);
|
||||||
|
|
||||||
|
await provider.search({ title: 'Batman' });
|
||||||
|
|
||||||
|
expect(client.searchIssues).toHaveBeenCalledWith('Batman', 'test-key');
|
||||||
|
expect(client.searchVolumes).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns empty array when searchIssues returns nothing', async () => {
|
||||||
|
vi.spyOn(client, 'searchIssues').mockResolvedValue([]);
|
||||||
|
|
||||||
|
expect(await provider.search({ title: 'Unknown Comic' })).toEqual([]);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
// search — throttle handling
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
describe('search - throttle handling', () => {
|
||||||
|
it('returns empty array and records throttle when client throws ProviderThrottleError', async () => {
|
||||||
|
vi.spyOn(client, 'searchVolumes').mockRejectedValue(new ProviderThrottleError());
|
||||||
|
vi.spyOn(client, 'windowResetMs').mockReturnValue(0);
|
||||||
|
|
||||||
|
const result = await provider.search({ title: 'Batman #1' });
|
||||||
|
|
||||||
|
expect(result).toEqual([]);
|
||||||
|
expect(throttleTracker.record).toHaveBeenCalledWith(MetadataProviderKey.COMICVINE, undefined);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('passes precise retryAfterSeconds derived from windowResetMs to the tracker', async () => {
|
||||||
|
vi.spyOn(client, 'searchVolumes').mockRejectedValue(new ProviderThrottleError());
|
||||||
|
vi.spyOn(client, 'windowResetMs').mockReturnValue(62_000); // 62 seconds remaining
|
||||||
|
|
||||||
|
await provider.search({ title: 'Batman #1' });
|
||||||
|
|
||||||
|
expect(throttleTracker.record).toHaveBeenCalledWith(MetadataProviderKey.COMICVINE, 62);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('ceils fractional seconds when computing retryAfterSeconds', async () => {
|
||||||
|
vi.spyOn(client, 'searchIssues').mockRejectedValue(new ProviderThrottleError());
|
||||||
|
vi.spyOn(client, 'windowResetMs').mockReturnValue(61_001); // 61.001 seconds
|
||||||
|
|
||||||
|
await provider.search({ title: 'Batman' });
|
||||||
|
|
||||||
|
expect(throttleTracker.record).toHaveBeenCalledWith(MetadataProviderKey.COMICVINE, 62);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('passes undefined retryAfterSeconds when windowResetMs returns 0', async () => {
|
||||||
|
vi.spyOn(client, 'searchIssues').mockRejectedValue(new ProviderThrottleError());
|
||||||
|
vi.spyOn(client, 'windowResetMs').mockReturnValue(0);
|
||||||
|
|
||||||
|
await provider.search({ title: 'Batman' });
|
||||||
|
|
||||||
|
expect(throttleTracker.record).toHaveBeenCalledWith(MetadataProviderKey.COMICVINE, undefined);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('rethrows non-throttle errors from search', async () => {
|
||||||
|
vi.spyOn(client, 'searchVolumes').mockRejectedValue(new Error('Unexpected'));
|
||||||
|
|
||||||
|
await expect(provider.search({ title: 'Batman #1' })).rejects.toThrow('Unexpected');
|
||||||
|
expect(throttleTracker.record).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('throttle on general path also records and returns empty array', async () => {
|
||||||
|
vi.spyOn(client, 'searchIssues').mockRejectedValue(new ProviderThrottleError());
|
||||||
|
vi.spyOn(client, 'windowResetMs').mockReturnValue(120_000);
|
||||||
|
|
||||||
|
const result = await provider.search({ title: 'Batman' });
|
||||||
|
|
||||||
|
expect(result).toEqual([]);
|
||||||
|
expect(throttleTracker.record).toHaveBeenCalledWith(MetadataProviderKey.COMICVINE, 120);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
// lookupById
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
describe('lookupById', () => {
|
||||||
|
it('returns null when disabled', async () => {
|
||||||
|
vi.spyOn(providerConfig, 'getConfig').mockResolvedValue({
|
||||||
|
...mockConfig,
|
||||||
|
comicvine: { enabled: false, apiKey: 'test-key' },
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(await provider.lookupById('100')).toBeNull();
|
||||||
|
expect(client.getIssueById).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns null when apiKey is missing', async () => {
|
||||||
|
vi.spyOn(providerConfig, 'getConfig').mockResolvedValue({
|
||||||
|
...mockConfig,
|
||||||
|
comicvine: { enabled: true, apiKey: '' },
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(await provider.lookupById('100')).toBeNull();
|
||||||
|
expect(client.getIssueById).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns mapped candidate when issue is found', async () => {
|
||||||
|
vi.spyOn(client, 'getIssueById').mockResolvedValue(mockIssue);
|
||||||
|
|
||||||
|
const result = await provider.lookupById('100');
|
||||||
|
|
||||||
|
expect(client.getIssueById).toHaveBeenCalledWith('100', 'test-key');
|
||||||
|
expect(result).not.toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns null when issue is not found', async () => {
|
||||||
|
vi.spyOn(client, 'getIssueById').mockResolvedValue(null);
|
||||||
|
|
||||||
|
expect(await provider.lookupById('999')).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns null and records throttle on ProviderThrottleError', async () => {
|
||||||
|
vi.spyOn(client, 'getIssueById').mockRejectedValue(new ProviderThrottleError());
|
||||||
|
vi.spyOn(client, 'windowResetMs').mockReturnValue(3_600_000);
|
||||||
|
|
||||||
|
const result = await provider.lookupById('100');
|
||||||
|
|
||||||
|
expect(result).toBeNull();
|
||||||
|
expect(throttleTracker.record).toHaveBeenCalledWith(MetadataProviderKey.COMICVINE, 3600);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('rethrows non-throttle errors', async () => {
|
||||||
|
vi.spyOn(client, 'getIssueById').mockRejectedValue(new Error('DB error'));
|
||||||
|
|
||||||
|
await expect(provider.lookupById('100')).rejects.toThrow('DB error');
|
||||||
|
expect(throttleTracker.record).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -2,6 +2,8 @@ import { Injectable, Logger } from '@nestjs/common';
|
|||||||
import { MetadataCandidate, MetadataProviderKey } from '@projectx/types';
|
import { MetadataCandidate, MetadataProviderKey } from '@projectx/types';
|
||||||
|
|
||||||
import { ProviderConfigService } from '../../../metadata-preferences/provider-config.service';
|
import { ProviderConfigService } from '../../../metadata-preferences/provider-config.service';
|
||||||
|
import { ProviderThrottleTracker } from '../../provider-throttle.tracker';
|
||||||
|
import { ProviderThrottleError } from '../../provider-throttle.error';
|
||||||
import { IdentifiableProvider } from '../metadata-provider';
|
import { IdentifiableProvider } from '../metadata-provider';
|
||||||
import { MetadataSearchParams } from '../metadata-search-params';
|
import { MetadataSearchParams } from '../metadata-search-params';
|
||||||
import { ComicVineClient } from './comicvine.client';
|
import { ComicVineClient } from './comicvine.client';
|
||||||
@@ -46,6 +48,7 @@ export class ComicVineProvider implements IdentifiableProvider {
|
|||||||
constructor(
|
constructor(
|
||||||
private readonly client: ComicVineClient,
|
private readonly client: ComicVineClient,
|
||||||
private readonly providerConfig: ProviderConfigService,
|
private readonly providerConfig: ProviderConfigService,
|
||||||
|
private readonly throttleTracker: ProviderThrottleTracker,
|
||||||
) {}
|
) {}
|
||||||
|
|
||||||
async search(params: MetadataSearchParams): Promise<MetadataCandidate[]> {
|
async search(params: MetadataSearchParams): Promise<MetadataCandidate[]> {
|
||||||
@@ -56,23 +59,41 @@ export class ComicVineProvider implements IdentifiableProvider {
|
|||||||
}
|
}
|
||||||
if (!params.title) return [];
|
if (!params.title) return [];
|
||||||
|
|
||||||
const maxCandidates = normalizeMaxCandidates(params.maxCandidatesPerProvider);
|
try {
|
||||||
const parsed = parseIssueTitle(params.title);
|
const maxCandidates = normalizeMaxCandidates(params.maxCandidatesPerProvider);
|
||||||
if (parsed) {
|
const parsed = parseIssueTitle(params.title);
|
||||||
return this.structuredSearch(parsed.seriesName, parsed.issueNumber, apiKey, maxCandidates);
|
return parsed
|
||||||
|
? await this.structuredSearch(parsed.seriesName, parsed.issueNumber, apiKey, maxCandidates)
|
||||||
|
: await this.generalSearch(params.title, apiKey, maxCandidates);
|
||||||
|
} catch (err) {
|
||||||
|
if (err instanceof ProviderThrottleError) {
|
||||||
|
this.recordThrottle();
|
||||||
|
return [];
|
||||||
|
}
|
||||||
|
throw err;
|
||||||
}
|
}
|
||||||
|
|
||||||
return this.generalSearch(params.title, apiKey, maxCandidates);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
async lookupById(providerId: string): Promise<MetadataCandidate | null> {
|
async lookupById(providerId: string): Promise<MetadataCandidate | null> {
|
||||||
const { enabled, apiKey } = await this.providerConfig.getConfig().then((c) => c.comicvine);
|
const { enabled, apiKey } = await this.providerConfig.getConfig().then((c) => c.comicvine);
|
||||||
if (!enabled || !apiKey) return null;
|
if (!enabled || !apiKey) return null;
|
||||||
|
|
||||||
const issue = await this.client.getIssueById(providerId, apiKey);
|
try {
|
||||||
if (!issue) return null;
|
const issue = await this.client.getIssueById(providerId, apiKey);
|
||||||
|
return issue ? mapIssueToCandidate(issue) : null;
|
||||||
|
} catch (err) {
|
||||||
|
if (err instanceof ProviderThrottleError) {
|
||||||
|
this.recordThrottle();
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
throw err;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
return mapIssueToCandidate(issue);
|
private recordThrottle(): void {
|
||||||
|
const waitMs = this.client.windowResetMs();
|
||||||
|
const retryAfterSeconds = waitMs > 0 ? Math.ceil(waitMs / 1000) : undefined;
|
||||||
|
this.throttleTracker.record(MetadataProviderKey.COMICVINE, retryAfterSeconds);
|
||||||
}
|
}
|
||||||
|
|
||||||
private async structuredSearch(seriesName: string, issueNumber: string, apiKey: string, maxCandidates: number): Promise<MetadataCandidate[]> {
|
private async structuredSearch(seriesName: string, issueNumber: string, apiKey: string, maxCandidates: number): Promise<MetadataCandidate[]> {
|
||||||
|
|||||||
Reference in New Issue
Block a user