fix: resolve final review findings (wiki cache tenant isolation, meeting search pagination, ambient test)

This commit is contained in:
Lucas Winther
2026-09-07 03:12:55 +02:00
parent e0aa91e388
commit 7097d1c174
8 changed files with 353 additions and 21 deletions
+7
View File
@@ -145,6 +145,13 @@ export class OpenProjectClient {
this.authHeader = `Basic ${btoa(`apikey:${this.apiKey}`)}`; this.authHeader = `Basic ${btoa(`apikey:${this.apiKey}`)}`;
} }
/**
* Returns a unique cache key scoped to both baseUrl and apiKey for multi-tenant cache isolation.
*/
getCacheKey(): string {
return `${this.baseUrl}#${this.apiKey}`;
}
/** /**
* Performs an HTTP GET request to OpenProject API v3. * Performs an HTTP GET request to OpenProject API v3.
*/ */
+35 -12
View File
@@ -378,21 +378,44 @@ export async function searchMeetings(
const opClient = resolveClient(client); const opClient = resolveClient(client);
const needle = params.query.toLowerCase().trim(); const needle = params.query.toLowerCase().trim();
// Fetch candidate meetings scoped to project if specified // Fetch candidate meetings scoped to project if specified, across multiple pages up to cap
const candidateBatchSize = Math.max(params.pageSize ?? 50, 50); const maxCandidates = 250;
const meetingsResult = await listMeetings( const candidateBatchSize = Math.min(maxCandidates, Math.max(params.pageSize ?? 50, 50));
{ const candidateMeetings: MeetingSummary[] = [];
projectId: params.projectId, let candidateOffset = 1;
offset: 1,
pageSize: candidateBatchSize,
},
opClient
);
const candidateMeetings = meetingsResult.elements; while (candidateMeetings.length < maxCandidates) {
const meetingsResult = await listMeetings(
{
projectId: params.projectId,
offset: candidateOffset,
pageSize: candidateBatchSize,
},
opClient
);
const elements = meetingsResult.elements;
if (elements.length === 0) {
break;
}
candidateMeetings.push(...elements);
// Stop if all meetings fetched or fewer elements returned than requested page size
if (
candidateMeetings.length >= meetingsResult.total ||
elements.length < candidateBatchSize
) {
break;
}
candidateOffset += 1;
}
const scopedCandidates = candidateMeetings.slice(0, maxCandidates);
// Process candidate meetings concurrently // Process candidate meetings concurrently
const matchPromises = candidateMeetings.map(async (meeting) => { const matchPromises = scopedCandidates.map(async (meeting) => {
const titleMatches = meeting.title.toLowerCase().includes(needle); const titleMatches = meeting.title.toLowerCase().includes(needle);
const locationMatches = Boolean( const locationMatches = Boolean(
meeting.location && meeting.location.toLowerCase().includes(needle) meeting.location && meeting.location.toLowerCase().includes(needle)
+1 -1
View File
@@ -76,7 +76,7 @@ export async function getOpenApiSpec(
client?: OpenProjectClient client?: OpenProjectClient
): Promise<unknown> { ): Promise<unknown> {
const activeClient = resolveClient(client); const activeClient = resolveClient(client);
const cacheKey = activeClient.baseUrl; const cacheKey = activeClient.getCacheKey?.() ?? activeClient.baseUrl;
if (options?.refresh) { if (options?.refresh) {
openApiCache.delete(cacheKey); openApiCache.delete(cacheKey);
+8 -8
View File
@@ -63,16 +63,16 @@ export interface SearchWikiPagesParams {
refreshCache?: boolean; refreshCache?: boolean;
} }
// In-memory cache for discovered wiki pages keyed by client baseUrl // In-memory cache for discovered wiki pages keyed by client cache key
const wikiPagesCache = new Map<string, Map<number, WikiPageDetail>>(); const wikiPagesCache = new Map<string, Map<number, WikiPageDetail>>();
const discoveredBaseUrls = new Set<string>(); const discoveredCacheKeys = new Set<string>();
/** /**
* Clears the in-memory wiki discovery cache. * Clears the in-memory wiki discovery cache.
*/ */
export function clearWikiCache(): void { export function clearWikiCache(): void {
wikiPagesCache.clear(); wikiPagesCache.clear();
discoveredBaseUrls.clear(); discoveredCacheKeys.clear();
} }
/** /**
@@ -209,8 +209,8 @@ export async function getWikiPage(
const detail = normalizeWikiPageDetail(pageResource, attachments); const detail = normalizeWikiPageDetail(pageResource, attachments);
// Update in-memory cache for this client baseUrl if initialized // Update in-memory cache for this client cache key if initialized
const cacheKey = opClient.baseUrl || "default"; const cacheKey = opClient.getCacheKey?.() ?? (opClient.baseUrl || "default");
let cacheMap = wikiPagesCache.get(cacheKey); let cacheMap = wikiPagesCache.get(cacheKey);
if (!cacheMap) { if (!cacheMap) {
cacheMap = new Map<number, WikiPageDetail>(); cacheMap = new Map<number, WikiPageDetail>();
@@ -259,10 +259,10 @@ export async function searchWikiPages(
client?: OpenProjectClient client?: OpenProjectClient
): Promise<WikiPageSummary[]> { ): Promise<WikiPageSummary[]> {
const opClient = resolveClient(client); const opClient = resolveClient(client);
const cacheKey = opClient.baseUrl || "default"; const cacheKey = opClient.getCacheKey?.() ?? (opClient.baseUrl || "default");
let cacheMap = wikiPagesCache.get(cacheKey); let cacheMap = wikiPagesCache.get(cacheKey);
const needsDiscovery = !cacheMap || !discoveredBaseUrls.has(cacheKey) || params?.refreshCache; const needsDiscovery = !cacheMap || !discoveredCacheKeys.has(cacheKey) || params?.refreshCache;
if (params?.refreshCache && cacheMap) { if (params?.refreshCache && cacheMap) {
cacheMap.clear(); cacheMap.clear();
@@ -355,7 +355,7 @@ export async function searchWikiPages(
} }
} }
discoveredBaseUrls.add(cacheKey); discoveredCacheKeys.add(cacheKey);
} }
// 3. Resolve target project ID if provided // 3. Resolve target project ID if provided
+20
View File
@@ -442,6 +442,26 @@ describe("OpenProjectClient (Mocked Unit Tests)", () => {
expect(capturedHeaders?.get("Accept")).toContain("application/hal+json"); expect(capturedHeaders?.get("Accept")).toContain("application/hal+json");
}); });
test("getCacheKey returns unique key scoped to both baseUrl and apiKey", () => {
const clientA = new OpenProjectClient({
baseUrl: "https://openproject.example.com",
apiKey: "user-key-a",
});
const clientB = new OpenProjectClient({
baseUrl: "https://openproject.example.com",
apiKey: "user-key-b",
});
const clientC = new OpenProjectClient({
baseUrl: "https://other.example.com",
apiKey: "user-key-a",
});
expect(clientA.getCacheKey()).toBe("https://openproject.example.com#user-key-a");
expect(clientB.getCacheKey()).toBe("https://openproject.example.com#user-key-b");
expect(clientC.getCacheKey()).toBe("https://other.example.com#user-key-a");
expect(clientA.getCacheKey()).not.toBe(clientB.getCacheKey());
});
test("sanitizes API credentials from error messages", async () => { test("sanitizes API credentials from error messages", async () => {
const secretKey = "super-secret-key-xyz-999"; const secretKey = "super-secret-key-xyz-999";
const mockFetch = async (): Promise<Response> => { const mockFetch = async (): Promise<Response> => {
+65
View File
@@ -252,6 +252,71 @@ describe("Meetings Service", () => {
expect(res.elements[0]!.meeting.id).toBe(2); expect(res.elements[0]!.meeting.id).toBe(2);
}); });
test("searchMeetings iterates candidate pages when total meetings exceed single page size", async () => {
const requestedOffsets: number[] = [];
const mockClient = {
get: async (path: string) => {
if (path.startsWith("/api/v3/meetings?")) {
const url = new URL(`http://dummy${path}`);
const offset = Number(url.searchParams.get("offset") ?? "1");
requestedOffsets.push(offset);
if (offset === 1) {
// Page 1: 50 meetings, none matching query
const page1Elements = Array.from({ length: 50 }, (_, i) => ({
_type: "Meeting",
id: i + 1,
title: `Daily Routine ${i + 1}`,
state: "open",
startTime: "2026-09-01T10:00:00Z",
endTime: "2026-09-01T10:30:00Z",
_links: { project: { href: "/api/v3/projects/1", title: "Demo" } },
}));
return {
_type: "Collection",
total: 55,
count: 50,
pageSize: 50,
offset: 1,
_embedded: { elements: page1Elements },
};
}
if (offset === 2) {
// Page 2: 5 meetings, meeting #52 has matching title
const page2Elements = Array.from({ length: 5 }, (_, i) => ({
_type: "Meeting",
id: 51 + i,
title: i === 1 ? "Special Architecture Sync" : `Daily Routine ${51 + i}`,
state: "open",
startTime: "2026-09-02T10:00:00Z",
endTime: "2026-09-02T10:30:00Z",
_links: { project: { href: "/api/v3/projects/1", title: "Demo" } },
}));
return {
_type: "Collection",
total: 55,
count: 5,
pageSize: 50,
offset: 2,
_embedded: { elements: page2Elements },
};
}
}
if (path.includes("/agenda_items")) {
return { _type: "Collection", total: 0, _embedded: { elements: [] } };
}
throw new Error(`Unexpected path: ${path}`);
},
} as unknown as OpenProjectClient;
const res = await searchMeetings({ query: "Architecture" }, mockClient);
expect(requestedOffsets).toEqual([1, 2]);
expect(res.total).toBe(1);
expect(res.elements[0]!.meeting.id).toBe(52);
expect(res.elements[0]!.meeting.title).toBe("Special Architecture Sync");
});
test("listMeetings supports time filter (upcoming and past)", async () => { test("listMeetings supports time filter (upcoming and past)", async () => {
let capturedPath = ""; let capturedPath = "";
const mockClient = { const mockClient = {
+177
View File
@@ -478,4 +478,181 @@ describe("Wikis Service", () => {
expect(refreshedResults).toHaveLength(1); expect(refreshedResults).toHaveLength(1);
expect(refreshedResults[0]!.id).toBe(1); expect(refreshedResults[0]!.id).toBe(1);
}); });
test("maintains isolated wiki page cache across different clients with same baseUrl but different API keys", async () => {
let clientACalls = 0;
let clientBCalls = 0;
const mockFetchA = async (input: string | URL | Request) => {
clientACalls++;
const url = String(input);
if (url.includes("/api/v3/wiki_pages/1/attachments")) {
return new Response(
JSON.stringify({ _type: "Collection", total: 0, _embedded: { elements: [] } }),
{ headers: { "Content-Type": "application/hal+json" } }
);
}
return new Response(
JSON.stringify({
_type: "WikiPage",
id: 1,
title: "User A Confidential Wiki",
_links: { project: { href: "/api/v3/projects/1", title: "Project A" } },
}),
{ headers: { "Content-Type": "application/hal+json" } }
);
};
const mockFetchB = async (input: string | URL | Request) => {
clientBCalls++;
const url = String(input);
if (url.includes("/api/v3/wiki_pages/1/attachments")) {
return new Response(
JSON.stringify({ _type: "Collection", total: 0, _embedded: { elements: [] } }),
{ headers: { "Content-Type": "application/hal+json" } }
);
}
return new Response(
JSON.stringify({
_type: "WikiPage",
id: 1,
title: "User B Confidential Wiki",
_links: { project: { href: "/api/v3/projects/2", title: "Project B" } },
}),
{ headers: { "Content-Type": "application/hal+json" } }
);
};
const clientA = new OpenProjectClient({
baseUrl: "https://openproject.example.com",
apiKey: "api-key-tenant-a",
fetchFn: mockFetchA,
});
const clientB = new OpenProjectClient({
baseUrl: "https://openproject.example.com",
apiKey: "api-key-tenant-b",
fetchFn: mockFetchB,
});
// Client A fetches wiki page 1 -> cached in client A's bucket
const pageA = await getWikiPage(1, clientA);
expect(pageA.title).toBe("User A Confidential Wiki");
expect(clientACalls).toBe(2);
// Client B fetches wiki page 1 -> must NOT use client A's cache
const pageB = await getWikiPage(1, clientB);
expect(pageB.title).toBe("User B Confidential Wiki");
expect(clientBCalls).toBe(2);
// Repeat fetch with client A -> returns User A data without refetching from server
const pageA2 = await getWikiPage(1, clientA);
expect(pageA2.title).toBe("User A Confidential Wiki");
});
test("maintains isolated wiki search discovery cache across different API keys", async () => {
const mockFetchA = async (input: string | URL | Request) => {
const url = String(input);
if (url.includes("/api/v3/wiki_page_links")) {
return new Response(
JSON.stringify({
_type: "Collection",
total: 1,
_embedded: {
elements: [
{
id: 101,
_links: { wikiPage: { href: "/api/v3/wiki_pages/10" } },
},
],
},
}),
{ headers: { "Content-Type": "application/hal+json" } }
);
}
if (url.includes("/api/v3/wiki_pages/10/attachments")) {
return new Response(
JSON.stringify({ _type: "Collection", total: 0, _embedded: { elements: [] } }),
{ headers: { "Content-Type": "application/hal+json" } }
);
}
if (url.includes("/api/v3/wiki_pages/10")) {
return new Response(
JSON.stringify({
_type: "WikiPage",
id: 10,
title: "Tenant A Page",
_links: { project: { href: "/api/v3/projects/1", title: "Project A" } },
}),
{ headers: { "Content-Type": "application/hal+json" } }
);
}
return new Response(JSON.stringify({ message: "Not found" }), {
status: 404,
headers: { "Content-Type": "application/json" },
});
};
const mockFetchB = async (input: string | URL | Request) => {
const url = String(input);
if (url.includes("/api/v3/wiki_page_links")) {
return new Response(
JSON.stringify({
_type: "Collection",
total: 1,
_embedded: {
elements: [
{
id: 201,
_links: { wikiPage: { href: "/api/v3/wiki_pages/20" } },
},
],
},
}),
{ headers: { "Content-Type": "application/hal+json" } }
);
}
if (url.includes("/api/v3/wiki_pages/20/attachments")) {
return new Response(
JSON.stringify({ _type: "Collection", total: 0, _embedded: { elements: [] } }),
{ headers: { "Content-Type": "application/hal+json" } }
);
}
if (url.includes("/api/v3/wiki_pages/20")) {
return new Response(
JSON.stringify({
_type: "WikiPage",
id: 20,
title: "Tenant B Page",
_links: { project: { href: "/api/v3/projects/2", title: "Project B" } },
}),
{ headers: { "Content-Type": "application/hal+json" } }
);
}
return new Response(JSON.stringify({ message: "Not found" }), {
status: 404,
headers: { "Content-Type": "application/json" },
});
};
const clientA = new OpenProjectClient({
baseUrl: "https://openproject.example.com",
apiKey: "api-key-tenant-a",
fetchFn: mockFetchA,
});
const clientB = new OpenProjectClient({
baseUrl: "https://openproject.example.com",
apiKey: "api-key-tenant-b",
fetchFn: mockFetchB,
});
const searchResultsA = await searchWikiPages({}, clientA);
expect(searchResultsA).toHaveLength(1);
expect(searchResultsA[0]!.title).toBe("Tenant A Page");
const searchResultsB = await searchWikiPages({}, clientB);
expect(searchResultsB).toHaveLength(1);
expect(searchResultsB[0]!.title).toBe("Tenant B Page");
});
}); });
+40
View File
@@ -1,5 +1,6 @@
import { describe, expect, test } from "bun:test"; import { describe, expect, test } from "bun:test";
import { OpenProjectClient } from "../src/client/api-client.ts"; import { OpenProjectClient } from "../src/client/api-client.ts";
import { runWithContext, type RequestContext } from "../src/context.ts";
import { listWorkPackageActivities } from "../src/services/work-packages.ts"; import { listWorkPackageActivities } from "../src/services/work-packages.ts";
describe("Work Package Activities Service", () => { describe("Work Package Activities Service", () => {
@@ -97,4 +98,43 @@ describe("Work Package Activities Service", () => {
const res = await listWorkPackageActivities({ workPackageId: 10 }, mockClient); const res = await listWorkPackageActivities({ workPackageId: 10 }, mockClient);
expect(res).toEqual([]); expect(res).toEqual([]);
}); });
test("resolves ambient RequestContext client when client parameter is omitted", async () => {
let calledPath = "";
const ambientClient = {
get: async (path: string) => {
calledPath = path;
return {
_type: "Collection",
total: 1,
count: 1,
_embedded: {
elements: [
{
id: 101,
version: 1,
createdAt: "2026-09-07T00:00:00Z",
comment: { raw: "Ambient resolution test comment" },
details: [],
},
],
},
};
},
} as unknown as OpenProjectClient;
const context: RequestContext = {
client: ambientClient,
isReadOnly: false,
};
await runWithContext(context, async () => {
const res = await listWorkPackageActivities({ workPackageId: 77 });
expect(calledPath).toBe("/api/v3/work_packages/77/activities");
expect(res).toHaveLength(1);
expect(res[0]!.id).toBe(101);
expect(res[0]!.comment).toBe("Ambient resolution test comment");
expect(res[0]!.isComment).toBe(true);
});
});
}); });