fix: finish review-driven single-source cleanup
This commit is contained in:
@@ -107,6 +107,19 @@ describe('accountMutationWorkflow', () => {
|
||||
expect(rebuildTokenRoutesFromAvailabilityMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('only marks refreshedModels when the refresh result explicitly says it refreshed', async () => {
|
||||
refreshModelsForAccountMock.mockResolvedValue({ accountId: 4, refreshed: false, status: 'skipped' });
|
||||
|
||||
const { convergeAccountMutation } = await import('./accountMutationWorkflow.js');
|
||||
const result = await convergeAccountMutation({
|
||||
accountId: 4,
|
||||
refreshModels: true,
|
||||
});
|
||||
|
||||
expect(result.modelRefreshResult).toEqual({ accountId: 4, refreshed: false, status: 'skipped' });
|
||||
expect(result.refreshedModels).toBe(false);
|
||||
});
|
||||
|
||||
it('refreshes model coverage in batches and maps failures', async () => {
|
||||
refreshModelsForAccountMock
|
||||
.mockResolvedValueOnce({ accountId: 1, refreshed: true, status: 'success' })
|
||||
@@ -129,4 +142,26 @@ describe('accountMutationWorkflow', () => {
|
||||
result: { createdRoutes: 2 },
|
||||
});
|
||||
});
|
||||
|
||||
it('rejects invalid batch sizes before starting the batch loop', async () => {
|
||||
const { refreshAccountCoverageBatch } = await import('./accountMutationWorkflow.js');
|
||||
|
||||
await expect(refreshAccountCoverageBatch({
|
||||
accountIds: [1, 2],
|
||||
batchSize: 1.5,
|
||||
mapFailure: (accountId, errorMessage) => ({ accountId, errorMessage }),
|
||||
})).rejects.toThrow('batchSize must be a positive integer');
|
||||
|
||||
expect(refreshModelsForAccountMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('exposes a best-effort route rebuild helper for controller callers', async () => {
|
||||
rebuildTokenRoutesFromAvailabilityMock.mockResolvedValueOnce({ createdRoutes: 1 });
|
||||
|
||||
const { rebuildRoutesBestEffort } = await import('./accountMutationWorkflow.js');
|
||||
await expect(rebuildRoutesBestEffort()).resolves.toBe(true);
|
||||
|
||||
rebuildTokenRoutesFromAvailabilityMock.mockRejectedValueOnce(new Error('rebuild failed'));
|
||||
await expect(rebuildRoutesBestEffort()).resolves.toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -20,6 +20,15 @@ export type CoverageBatchRebuildResult =
|
||||
| { success: true; result: Awaited<ReturnType<typeof rebuildTokenRoutesFromAvailability>> }
|
||||
| { success: false; error: string };
|
||||
|
||||
export async function rebuildRoutesBestEffort(): Promise<boolean> {
|
||||
try {
|
||||
await rebuildTokenRoutesFromAvailability();
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
export async function convergeAccountMutation(input: {
|
||||
accountId: number;
|
||||
preferredApiToken?: string | null;
|
||||
@@ -110,7 +119,7 @@ export async function convergeAccountMutation(input: {
|
||||
const modelRefreshResult = await runStep(() => refreshModelsForAccount(input.accountId));
|
||||
if (modelRefreshResult) {
|
||||
result.modelRefreshResult = modelRefreshResult;
|
||||
result.refreshedModels = true;
|
||||
result.refreshedModels = modelRefreshResult.refreshed === true;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -133,6 +142,11 @@ export async function refreshAccountCoverageBatch<TFailure>(input: {
|
||||
refresh: Array<ModelRefreshResult | TFailure>;
|
||||
rebuild: CoverageBatchRebuildResult | null;
|
||||
}> {
|
||||
if (!Number.isInteger(input.batchSize) || input.batchSize <= 0) {
|
||||
throw new Error('batchSize must be a positive integer');
|
||||
}
|
||||
|
||||
const batchSize = input.batchSize;
|
||||
const uniqueAccountIds = Array.from(new Set(
|
||||
input.accountIds.filter((id) => Number.isFinite(id) && id > 0),
|
||||
));
|
||||
@@ -142,8 +156,8 @@ export async function refreshAccountCoverageBatch<TFailure>(input: {
|
||||
}
|
||||
|
||||
const refresh: Array<ModelRefreshResult | TFailure> = [];
|
||||
for (let offset = 0; offset < uniqueAccountIds.length; offset += input.batchSize) {
|
||||
const batch = uniqueAccountIds.slice(offset, offset + input.batchSize);
|
||||
for (let offset = 0; offset < uniqueAccountIds.length; offset += batchSize) {
|
||||
const batch = uniqueAccountIds.slice(offset, offset + batchSize);
|
||||
const settled = await Promise.allSettled(
|
||||
batch.map(async (accountId) => refreshModelsForAccount(accountId)),
|
||||
);
|
||||
|
||||
@@ -7,6 +7,7 @@ import {
|
||||
|
||||
class TestStandardApiProviderAdapter extends StandardApiProviderAdapterBase {
|
||||
readonly platformName = 'test-standard';
|
||||
fetchJsonImpl = async () => ({ data: [] as Array<{ id: string }> });
|
||||
|
||||
async detect(_url: string): Promise<boolean> {
|
||||
return false;
|
||||
@@ -15,6 +16,14 @@ class TestStandardApiProviderAdapter extends StandardApiProviderAdapterBase {
|
||||
async getModels(_baseUrl: string, _token: string): Promise<string[]> {
|
||||
return [];
|
||||
}
|
||||
|
||||
protected override async fetchJson<T>(url: string, options?: Parameters<StandardApiProviderAdapterBase['fetchJson']>[1]): Promise<T> {
|
||||
return this.fetchJsonImpl(url, options) as Promise<T>;
|
||||
}
|
||||
|
||||
async fetchModelsForTest(options: Parameters<StandardApiProviderAdapterBase['fetchModelsFromStandardEndpoint']>[0]) {
|
||||
return this.fetchModelsFromStandardEndpoint(options);
|
||||
}
|
||||
}
|
||||
|
||||
describe('standardApiProvider helpers', () => {
|
||||
@@ -43,4 +52,33 @@ describe('standardApiProvider helpers', () => {
|
||||
quota: 0,
|
||||
});
|
||||
});
|
||||
|
||||
it('does not swallow mapper bugs while still returning empty lists for network failures', async () => {
|
||||
const adapter = new TestStandardApiProviderAdapter();
|
||||
adapter.fetchJsonImpl = async () => ({ data: [{ id: 'gpt-5' }] });
|
||||
|
||||
await expect(adapter.fetchModelsForTest({
|
||||
baseUrl: 'https://api.example.com',
|
||||
mapResponse: () => {
|
||||
throw new Error('mapper exploded');
|
||||
},
|
||||
})).rejects.toThrow('mapper exploded');
|
||||
|
||||
adapter.fetchJsonImpl = async () => {
|
||||
throw new Error('network failed');
|
||||
};
|
||||
|
||||
await expect(adapter.fetchModelsForTest({
|
||||
baseUrl: 'https://api.example.com',
|
||||
})).resolves.toEqual([]);
|
||||
});
|
||||
|
||||
it('rejects invalid payload shapes instead of silently treating them as no models', async () => {
|
||||
const adapter = new TestStandardApiProviderAdapter();
|
||||
adapter.fetchJsonImpl = async () => ({ data: 'not-an-array' });
|
||||
|
||||
await expect(adapter.fetchModelsForTest({
|
||||
baseUrl: 'https://api.example.com',
|
||||
})).rejects.toThrow('invalid standard models payload');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -60,18 +60,27 @@ export abstract class StandardApiProviderAdapterBase extends BasePlatformAdapter
|
||||
? options.resolveUrl(normalizedBaseUrl)
|
||||
: resolveVersionedModelsUrl(normalizedBaseUrl);
|
||||
|
||||
let payload: any;
|
||||
try {
|
||||
const payload = await this.fetchJson<any>(url, {
|
||||
payload = await this.fetchJson<any>(url, {
|
||||
headers: options.headers,
|
||||
});
|
||||
const rows = options.mapResponse
|
||||
? options.mapResponse(payload)
|
||||
: (payload?.data || []).map((item: any) => item?.id);
|
||||
return rows
|
||||
.map((item) => (typeof item === 'string' ? item.trim() : ''))
|
||||
.filter((item) => item.length > 0);
|
||||
} catch {
|
||||
return [];
|
||||
}
|
||||
|
||||
const rows = options.mapResponse
|
||||
? options.mapResponse(payload)
|
||||
: Array.isArray(payload?.data)
|
||||
? payload.data.map((item: any) => item?.id)
|
||||
: null;
|
||||
|
||||
if (!Array.isArray(rows)) {
|
||||
throw new Error('invalid standard models payload');
|
||||
}
|
||||
|
||||
return rows
|
||||
.map((item) => (typeof item === 'string' ? item.trim() : ''))
|
||||
.filter((item) => item.length > 0);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user