Skip to content

Commit 2788a06

Browse files
authored
fix: address code review feedback - safer array access, clearer naming
Agent-Logs-Url: https://github.com/counterfact/api-simulator/sessions/e45b6211-c18d-4a86-8bf3-8bef0d04957b
1 parent e45782e commit 2788a06

2 files changed

Lines changed: 39 additions & 16 deletions

File tree

src/app.ts

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -254,11 +254,15 @@ function buildMultiSpecMiddleware(
254254
return async function multiSpecMiddleware(ctx, next) {
255255
for (const { prefix, middleware } of specMiddlewares) {
256256
if (ctx.request.path.startsWith(prefix)) {
257-
let passedThrough = false;
257+
// The per-spec koaMiddleware calls `next` only when the path does NOT
258+
// match its routePrefix, which means we should move on to the next spec.
259+
// When it handles the request it does NOT call next, so `calledNext`
260+
// stays false and we return immediately.
261+
let calledNext = false;
258262
await middleware(ctx, async () => {
259-
passedThrough = true;
263+
calledNext = true;
260264
});
261-
if (!passedThrough) return;
265+
if (!calledNext) return;
262266
}
263267
}
264268
await next();
@@ -307,19 +311,22 @@ export async function counterfact(config: Config) {
307311
),
308312
);
309313

314+
// Guaranteed non-empty: we checked `config.specs.length > 0` above and
315+
// `specBundles` has the same length as `config.specs`.
316+
const primaryBundle = specBundles[0] as SpecBundle;
317+
const primarySpec = config.specs[0] as (typeof config.specs)[number];
318+
310319
const compositeMiddleware = buildMultiSpecMiddleware(specBundles, config);
311320

312321
// Use the first spec's registry for the Koa admin/OpenAPI UI (best effort).
313-
const primaryRegistry = specBundles[0]!.registry;
322+
const primaryRegistry = primaryBundle.registry;
314323

315324
const koaApp = createKoaApp(
316325
primaryRegistry,
317326
compositeMiddleware,
318327
{
319328
...config,
320-
openApiPath: specBundles[0]!.openApiDocument
321-
? config.specs[0]!.source
322-
: "_",
329+
openApiPath: primaryBundle.openApiDocument ? primarySpec.source : "_",
323330
},
324331
contextRegistry,
325332
);
@@ -359,7 +366,7 @@ export async function counterfact(config: Config) {
359366
scenarioRegistry,
360367
contextRegistry,
361368
config,
362-
specBundles[0]!.openApiDocument,
369+
primaryBundle.openApiDocument,
363370
);
364371

365372
const server = koaApp.listen({ port: config.port });
@@ -401,7 +408,7 @@ export async function counterfact(config: Config) {
401408
primaryRegistry,
402409
config,
403410
undefined,
404-
specBundles[0]!.openApiDocument,
411+
primaryBundle.openApiDocument,
405412
scenarioRegistry,
406413
),
407414
};

test/app.test.ts

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -255,32 +255,48 @@ describe("counterfact multi-spec mode", () => {
255255

256256
// Simulate a Koa context for /alpha/...
257257
const alphaCtx = {
258-
request: { path: "/alpha/hello/world", method: "GET", body: undefined, headers: {}, query: {} },
258+
request: {
259+
path: "/alpha/hello/world",
260+
method: "GET",
261+
body: undefined,
262+
headers: {},
263+
query: {},
264+
},
259265
req: {},
260266
status: 200,
261267
set: () => {},
262268
body: undefined,
263269
} as any;
264270

265271
let alphaCalledNext = false;
266-
await result.koaMiddleware(alphaCtx, async () => { alphaCalledNext = true; });
272+
await result.koaMiddleware(alphaCtx, async () => {
273+
alphaCalledNext = true;
274+
});
267275

268-
// The request should have been handled (next was NOT called or it was passed through normally)
269-
// Since /alpha/hello/world may not exist in the registry, it returns a 404-like response
270-
// but the middleware should NOT propagate to next() unless the prefix doesn't match.
276+
// /alpha matches the "alpha" spec prefix. The per-spec middleware handles
277+
// any request under that prefix (even if the route doesn't exist) and does
278+
// NOT call next(), so alphaCalledNext should remain false.
271279
expect(alphaCalledNext).toBe(false);
272280

273281
// Simulate a Koa context for an unknown prefix /gamma/...
274282
const unknownCtx = {
275-
request: { path: "/gamma/something", method: "GET", body: undefined, headers: {}, query: {} },
283+
request: {
284+
path: "/gamma/something",
285+
method: "GET",
286+
body: undefined,
287+
headers: {},
288+
query: {},
289+
},
276290
req: {},
277291
status: 200,
278292
set: () => {},
279293
body: undefined,
280294
} as any;
281295

282296
let unknownCalledNext = false;
283-
await result.koaMiddleware(unknownCtx, async () => { unknownCalledNext = true; });
297+
await result.koaMiddleware(unknownCtx, async () => {
298+
unknownCalledNext = true;
299+
});
284300

285301
// No spec matches /gamma, so next should be called
286302
expect(unknownCalledNext).toBe(true);

0 commit comments

Comments
 (0)