Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add "requestId" as response resolver argument #1942

Merged
merged 6 commits into from
Jan 15, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/core/handlers/RequestHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ export interface RequestHandlerExecutionResult<
handler: RequestHandler
parsedResult?: ParsedResult
request: Request
requestId: string
response?: Response
}

Expand Down Expand Up @@ -215,6 +216,7 @@ export abstract class RequestHandler<
*/
public async run(args: {
request: StrictRequest<any>
requestId: string
resolutionContext?: ResponseResolutionContext
}): Promise<RequestHandlerExecutionResult<ParsedResult> | null> {
if (this.isUsed && this.options?.once) {
Expand Down Expand Up @@ -261,12 +263,14 @@ export abstract class RequestHandler<
const mockedResponse = (await executeResolver({
...resolverExtras,
request: args.request,
requestId: args.requestId,
})) as Response

const executionResult = this.createExecutionResult({
// Pass the cloned request to the result so that logging
// and other consumers could read its body once more.
request: requestClone,
requestId: args.requestId,
response: mockedResponse,
parsedResult,
})
Expand Down Expand Up @@ -325,12 +329,14 @@ export abstract class RequestHandler<

private createExecutionResult(args: {
request: Request
requestId: string
parsedResult: ParsedResult
response?: Response
}): RequestHandlerExecutionResult<ParsedResult> {
return {
handler: this,
request: args.request,
requestId: args.requestId,
response: args.response,
parsedResult: args.parsedResult,
}
Expand Down
18 changes: 12 additions & 6 deletions src/core/utils/getResponse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,22 @@ export interface ResponseResolutionContext {
/**
* Returns a mocked response for a given request using following request handlers.
*/
export const getResponse = async <Handler extends Array<RequestHandler>>(
request: Request,
handlers: Handler,
resolutionContext?: ResponseResolutionContext,
): Promise<ResponseLookupResult | null> => {
export const getResponse = async <Handler extends Array<RequestHandler>>({
request,
requestId,
handlers,
resolutionContext,
}: {
request: Request
requestId: string
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only thing I'm not sure about here is whether requiring requestId for the .run() method is right.

While this is not widely used, you can call a request handler directly using its .run() method. Just accepting request makes sense. Demanding requestId means the consumer has to contrive that ID. Hmm.

I think we can make this required and suggest using crypto.randomUUID() as a placeholder value if someone has to call a handler directly. If you want to—you must provide a request ID.

handlers: Handler
resolutionContext?: ResponseResolutionContext
}): Promise<ResponseLookupResult | null> => {
let matchingHandler: RequestHandler | null = null
let result: RequestHandlerExecutionResult<any> | null = null

for (const handler of handlers) {
result = await handler.run({ request, resolutionContext })
result = await handler.run({ request, requestId, resolutionContext })

// If the handler produces some result for this request,
// it automatically becomes matching.
Expand Down
20 changes: 20 additions & 0 deletions src/core/utils/handleRequest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,26 @@ it('returns undefined without warning on a passthrough request', async () => {
expect(handleRequestOptions.onMockedResponse).not.toHaveBeenCalled()
})

it('calls the handler with the requestId', async () => {
const { emitter } = setup()

const requestId = uuidv4()
const request = new Request(new URL('http://localhost/user'))
const handlerFn = vi.fn()
const handlers: Array<RequestHandler> = [http.get('/user', handlerFn)]

await handleRequest(
request,
requestId,
handlers,
options,
emitter,
handleRequestOptions,
)

expect(handlerFn).toHaveBeenCalledWith(expect.objectContaining({ requestId }))
})

it('marks the first matching one-time handler as used', async () => {
const { emitter } = setup()

Expand Down
7 changes: 4 additions & 3 deletions src/core/utils/handleRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,12 @@ export async function handleRequest(

// Resolve a mocked response from the list of request handlers.
const lookupResult = await until(() => {
return getResponse(
return getResponse({
request,
requestId,
handlers,
handleRequestOptions?.resolutionContext,
)
resolutionContext: handleRequestOptions?.resolutionContext,
})
})

if (lookupResult.error) {
Expand Down