refactor(fts): address PR #488 review concerns
- Extract isPureTextRelevance() private static method to replace the 7-clause inline boolean in searchDocuments - Guard long→int cast in relevanceSortedPageFromSql to prevent silent overflow at page ≥43M (CWE-190) - resolvePersonName now uses the typed API client (createApiClient) instead of raw fetch, aligning with project conventions - Update DocumentServiceTest stubs to match new FTS path (findFtsPageRaw + findAllById instead of findAllMatchingIdsByFts) - Rewrite page.server.spec.ts person-name tests to mock via path-based API dispatch, matching the new api.GET call site Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -5,14 +5,17 @@ import type { components } from '$lib/generated/api';
|
||||
|
||||
const UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
|
||||
|
||||
async function resolvePersonName(id: string, fetch: typeof globalThis.fetch): Promise<string> {
|
||||
async function resolvePersonName(
|
||||
id: string,
|
||||
api: ReturnType<typeof createApiClient>
|
||||
): Promise<string> {
|
||||
if (!UUID_RE.test(id)) return '';
|
||||
try {
|
||||
const res = await fetch(`/api/persons/${id}`);
|
||||
if (!res.ok) return '';
|
||||
const person = await res.json();
|
||||
return person.displayName ?? '';
|
||||
} catch {
|
||||
const result = await api.GET('/api/persons/{id}', { params: { path: { id } } });
|
||||
if (!result.response.ok) return '';
|
||||
return result.data?.displayName ?? '';
|
||||
} catch (e) {
|
||||
console.error('[resolvePersonName] failed for id', id, e);
|
||||
return '';
|
||||
}
|
||||
}
|
||||
@@ -70,7 +73,7 @@ export async function load({ url, fetch }) {
|
||||
}
|
||||
}
|
||||
}),
|
||||
Promise.all([resolvePersonName(senderId, fetch), resolvePersonName(receiverId, fetch)])
|
||||
Promise.all([resolvePersonName(senderId, api), resolvePersonName(receiverId, api)])
|
||||
]);
|
||||
} catch {
|
||||
return {
|
||||
|
||||
@@ -171,66 +171,70 @@ describe('documents page load — network error fallback', () => {
|
||||
// ─── person name resolution ───────────────────────────────────────────────────
|
||||
|
||||
describe('documents page load — person name resolution', () => {
|
||||
function makeSearchMock() {
|
||||
const mockGet = vi.fn().mockResolvedValue({
|
||||
response: { ok: true, status: 200 },
|
||||
data: { items: [], totalElements: 0, pageNumber: 0, pageSize: 50, totalPages: 0 }
|
||||
function makeSearchMock(personResult?: { ok: boolean; displayName?: string }) {
|
||||
const mockGet = vi.fn().mockImplementation((path: string) => {
|
||||
if (path === '/api/documents/search') {
|
||||
return Promise.resolve({
|
||||
response: { ok: true, status: 200 },
|
||||
data: { items: [], totalElements: 0, pageNumber: 0, pageSize: 50, totalPages: 0 }
|
||||
});
|
||||
}
|
||||
// person lookup via api.GET('/api/persons/{id}', ...)
|
||||
if (!personResult?.ok) {
|
||||
return Promise.resolve({ response: { ok: false, status: 404 }, data: undefined });
|
||||
}
|
||||
return Promise.resolve({
|
||||
response: { ok: true, status: 200 },
|
||||
data: { displayName: personResult.displayName ?? '' }
|
||||
});
|
||||
});
|
||||
vi.mocked(createApiClient).mockReturnValue({ GET: mockGet } as ReturnType<
|
||||
typeof createApiClient
|
||||
>);
|
||||
return mockGet;
|
||||
}
|
||||
|
||||
it('returns initialSenderName from person lookup when senderId is a valid UUID', async () => {
|
||||
makeSearchMock();
|
||||
const mockFetch = vi.fn().mockResolvedValue({
|
||||
ok: true,
|
||||
json: vi.fn().mockResolvedValue({ displayName: 'Max Mustermann' })
|
||||
});
|
||||
makeSearchMock({ ok: true, displayName: 'Max Mustermann' });
|
||||
|
||||
const result = await load({
|
||||
url: makeUrl({ senderId: '11111111-1111-1111-1111-111111111111' }),
|
||||
fetch: mockFetch as unknown as typeof fetch
|
||||
fetch: vi.fn() as unknown as typeof fetch
|
||||
});
|
||||
|
||||
expect(result.initialSenderName).toBe('Max Mustermann');
|
||||
});
|
||||
|
||||
it('returns initialReceiverName from person lookup when receiverId is a valid UUID', async () => {
|
||||
makeSearchMock();
|
||||
const mockFetch = vi.fn().mockResolvedValue({
|
||||
ok: true,
|
||||
json: vi.fn().mockResolvedValue({ displayName: 'Anna Musterfrau' })
|
||||
});
|
||||
makeSearchMock({ ok: true, displayName: 'Anna Musterfrau' });
|
||||
|
||||
const result = await load({
|
||||
url: makeUrl({ receiverId: '22222222-2222-2222-2222-222222222222' }),
|
||||
fetch: mockFetch as unknown as typeof fetch
|
||||
fetch: vi.fn() as unknown as typeof fetch
|
||||
});
|
||||
|
||||
expect(result.initialReceiverName).toBe('Anna Musterfrau');
|
||||
});
|
||||
|
||||
it('returns empty string when senderId is not a valid UUID', async () => {
|
||||
makeSearchMock();
|
||||
const mockFetch = vi.fn();
|
||||
const mockGet = makeSearchMock();
|
||||
|
||||
const result = await load({
|
||||
url: makeUrl({ senderId: 'not-a-uuid' }),
|
||||
fetch: mockFetch as unknown as typeof fetch
|
||||
fetch: vi.fn() as unknown as typeof fetch
|
||||
});
|
||||
|
||||
expect(result.initialSenderName).toBe('');
|
||||
expect(mockFetch).not.toHaveBeenCalledWith(expect.stringContaining('/api/persons/'));
|
||||
// UUID guard fires before any api.GET call — only document search is called
|
||||
expect(mockGet).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('returns empty string when person fetch returns 404', async () => {
|
||||
makeSearchMock();
|
||||
const mockFetch = vi.fn().mockResolvedValue({ ok: false, status: 404 });
|
||||
it('returns empty string when person api returns 404', async () => {
|
||||
makeSearchMock({ ok: false });
|
||||
|
||||
const result = await load({
|
||||
url: makeUrl({ senderId: '11111111-1111-1111-1111-111111111111' }),
|
||||
fetch: mockFetch as unknown as typeof fetch
|
||||
fetch: vi.fn() as unknown as typeof fetch
|
||||
});
|
||||
|
||||
expect(result.initialSenderName).toBe('');
|
||||
|
||||
Reference in New Issue
Block a user