From 40b79f2e611dc5b8b5801595acf3cb17bef785fe Mon Sep 17 00:00:00 2001 From: Koen Vlaswinkel Date: Fri, 29 Sep 2023 14:08:34 +0200 Subject: [PATCH] Improve scenario recording --- .../src/common/mock-gh-api/gh-api-request.ts | 24 ++++--- .../common/mock-gh-api/mock-gh-api-server.ts | 18 ++++- .../src/common/mock-gh-api/recorder.ts | 67 +++++-------------- .../vscode/vscode-mock-gh-api-server.ts | 8 ++- .../gh-api/gh-api-client.test.ts | 1 + ...nt-analysis-submission-integration.test.ts | 1 + 6 files changed, 54 insertions(+), 65 deletions(-) diff --git a/extensions/ql-vscode/src/common/mock-gh-api/gh-api-request.ts b/extensions/ql-vscode/src/common/mock-gh-api/gh-api-request.ts index a14ac6423..7d3ae7759 100644 --- a/extensions/ql-vscode/src/common/mock-gh-api/gh-api-request.ts +++ b/extensions/ql-vscode/src/common/mock-gh-api/gh-api-request.ts @@ -74,6 +74,13 @@ export interface GetVariantAnalysisRepoResultRequest { }; } +interface CodeSearchResponse { + total_count: number; + items: Array<{ + repository: Repository; + }>; +} + interface CodeSearchRequest { request: { kind: RequestKind.CodeSearch; @@ -81,16 +88,14 @@ interface CodeSearchRequest { }; response: { status: number; - body?: { - total_count?: number; - items?: Array<{ - repository: Repository; - }>; - }; - message?: string; + body?: CodeSearchResponse | BasicErorResponse; }; } +interface AutoModelResponse { + models: string; +} + interface AutoModelRequest { request: { kind: RequestKind.AutoModel; @@ -100,10 +105,7 @@ interface AutoModelRequest { }; response: { status: number; - body?: { - models: string; - }; - message?: string; + body?: AutoModelResponse | BasicErorResponse; }; } diff --git a/extensions/ql-vscode/src/common/mock-gh-api/mock-gh-api-server.ts b/extensions/ql-vscode/src/common/mock-gh-api/mock-gh-api-server.ts index a0bb9408e..9f32583ee 100644 --- a/extensions/ql-vscode/src/common/mock-gh-api/mock-gh-api-server.ts +++ b/extensions/ql-vscode/src/common/mock-gh-api/mock-gh-api-server.ts @@ -12,18 +12,31 @@ import { getDirectoryNamesInsidePath } from "../files"; * Enables mocking of the GitHub API server via HTTP interception, using msw. */ export class MockGitHubApiServer extends DisposableObject { + private _isListening: boolean; + private readonly server: SetupServer; private readonly recorder: Recorder; constructor() { super(); + this._isListening = false; this.server = setupServer(); this.recorder = this.push(new Recorder(this.server)); } + public startServer(): void { + if (this._isListening) { + return; + } + + this.server.listen({ onUnhandledRequest: "bypass" }); + this._isListening = true; + } + public stopServer(): void { this.server.close(); + this._isListening = false; } public async loadScenario( @@ -42,7 +55,6 @@ export class MockGitHubApiServer extends DisposableObject { const handlers = await createRequestHandlers(scenarioPath); this.server.resetHandlers(); this.server.use(...handlers); - this.server.listen({ onUnhandledRequest: "bypass" }); } public async saveScenario( @@ -99,6 +111,10 @@ export class MockGitHubApiServer extends DisposableObject { return await getDirectoryNamesInsidePath(scenariosPath); } + public get isListening(): boolean { + return this._isListening; + } + public get isRecording(): boolean { return this.recorder.isRecording; } diff --git a/extensions/ql-vscode/src/common/mock-gh-api/recorder.ts b/extensions/ql-vscode/src/common/mock-gh-api/recorder.ts index b8e903c49..3854a0988 100644 --- a/extensions/ql-vscode/src/common/mock-gh-api/recorder.ts +++ b/extensions/ql-vscode/src/common/mock-gh-api/recorder.ts @@ -3,8 +3,6 @@ import { join } from "path"; import { SetupServer } from "msw/node"; -import fetch from "node-fetch"; - import { DisposableObject } from "../disposable-object"; import { @@ -14,14 +12,12 @@ import { } from "./gh-api-request"; export class Recorder extends DisposableObject { - private readonly allRequests = new Map(); private currentRecordedScenario: GitHubApiRequest[] = []; private _isRecording = false; constructor(private readonly server: SetupServer) { super(); - this.onRequestStart = this.onRequestStart.bind(this); this.onResponseBypass = this.onResponseBypass.bind(this); } @@ -42,7 +38,6 @@ export class Recorder extends DisposableObject { this.clear(); - this.server.events.on("request:start", this.onRequestStart); this.server.events.on("response:bypass", this.onResponseBypass); } @@ -53,13 +48,11 @@ export class Recorder extends DisposableObject { this._isRecording = false; - this.server.events.removeListener("request:start", this.onRequestStart); this.server.events.removeListener("response:bypass", this.onResponseBypass); } public clear() { this.currentRecordedScenario = []; - this.allRequests.clear(); } public async save(scenariosPath: string, name: string): Promise { @@ -109,34 +102,14 @@ export class Recorder extends DisposableObject { return scenarioDirectory; } - private onRequestStart(request: Request, requestId: string): void { - if (request.headers.has("x-vscode-codeql-msw-bypass")) { - return; - } - - this.allRequests.set(requestId, request); - } - private async onResponseBypass( response: Response, - _: Request, - requestId: string, + request: Request, + _requestId: string, ): Promise { - const request = this.allRequests.get(requestId); - this.allRequests.delete(requestId); - if (!request) { - return; - } - - if (response.body === undefined) { - return; - } - const gitHubApiRequest = await createGitHubApiRequest( - request.url.toString(), - response.status, - response.body?.toString() || "", - response.headers, + request.url, + response, ); if (!gitHubApiRequest) { return; @@ -148,14 +121,15 @@ export class Recorder extends DisposableObject { async function createGitHubApiRequest( url: string, - status: number, - body: string, - headers: globalThis.Headers, + response: Response, ): Promise { if (!url) { return undefined; } + const status = response.status; + const headers = response.headers; + if (url.match(/\/repos\/[a-zA-Z0-9-_.]+\/[a-zA-Z0-9-_.]+$/)) { return { request: { @@ -163,7 +137,7 @@ async function createGitHubApiRequest( }, response: { status, - body: JSON.parse(body), + body: await response.json(), }, }; } @@ -177,7 +151,7 @@ async function createGitHubApiRequest( }, response: { status, - body: JSON.parse(body), + body: await response.json(), }, }; } @@ -193,7 +167,7 @@ async function createGitHubApiRequest( }, response: { status, - body: JSON.parse(body), + body: await response.json(), }, }; } @@ -209,7 +183,7 @@ async function createGitHubApiRequest( }, response: { status, - body: JSON.parse(body), + body: await response.json(), }, }; } @@ -219,17 +193,6 @@ async function createGitHubApiRequest( /objects-origin\.githubusercontent\.com\/codeql-query-console\/codeql-variant-analysis-repo-tasks\/\d+\/(?\d+)/, ); if (repoDownloadMatch?.groups?.repositoryId) { - // msw currently doesn't support binary response bodies, so we need to download this separately - // see https://github.com/mswjs/interceptors/blob/15eafa6215a328219999403e3ff110e71699b016/src/interceptors/ClientRequest/utils/getIncomingMessageBody.ts#L24-L33 - // Essentially, mws is trying to decode a ZIP file as UTF-8 which changes the bytes and corrupts the file. - const response = await fetch(url, { - headers: { - // We need to ensure we don't end up in an infinite loop, since this request will also be intercepted - "x-vscode-codeql-msw-bypass": "true", - }, - }); - const responseBuffer = await response.buffer(); - return { request: { kind: RequestKind.GetVariantAnalysisRepoResult, @@ -237,7 +200,7 @@ async function createGitHubApiRequest( }, response: { status, - body: responseBuffer, + body: Buffer.from(await response.arrayBuffer()), contentType: headers.get("content-type") ?? "application/octet-stream", }, }; @@ -252,7 +215,7 @@ async function createGitHubApiRequest( }, response: { status, - body: JSON.parse(body), + body: await response.json(), }, }; } @@ -267,7 +230,7 @@ async function createGitHubApiRequest( }, response: { status, - body: JSON.parse(body), + body: await response.json(), }, }; } diff --git a/extensions/ql-vscode/src/common/mock-gh-api/vscode/vscode-mock-gh-api-server.ts b/extensions/ql-vscode/src/common/mock-gh-api/vscode/vscode-mock-gh-api-server.ts index 9d4d5dcac..aae2fa8ca 100644 --- a/extensions/ql-vscode/src/common/mock-gh-api/vscode/vscode-mock-gh-api-server.ts +++ b/extensions/ql-vscode/src/common/mock-gh-api/vscode/vscode-mock-gh-api-server.ts @@ -42,6 +42,10 @@ export class VSCodeMockGitHubApiServer extends DisposableObject { }; } + public async startServer(): Promise { + this.server.startServer(); + } + public async stopServer(): Promise { this.server.stopServer(); @@ -252,7 +256,9 @@ export class VSCodeMockGitHubApiServer extends DisposableObject { } private async onConfigChange(): Promise { - if (!this.config.mockServerEnabled) { + if (this.config.mockServerEnabled && !this.server.isListening) { + await this.startServer(); + } else if (!this.config.mockServerEnabled && this.server.isListening) { await this.stopServer(); } } diff --git a/extensions/ql-vscode/test/unit-tests/variant-analysis/gh-api/gh-api-client.test.ts b/extensions/ql-vscode/test/unit-tests/variant-analysis/gh-api/gh-api-client.test.ts index 351d22dda..b03b107cb 100644 --- a/extensions/ql-vscode/test/unit-tests/variant-analysis/gh-api/gh-api-client.test.ts +++ b/extensions/ql-vscode/test/unit-tests/variant-analysis/gh-api/gh-api-client.test.ts @@ -13,6 +13,7 @@ import { response as variantAnalysisRepoJson_response } from "../../../../src/co import { testCredentialsWithRealOctokit } from "../../../factories/authentication"; const mockServer = new MockGitHubApiServer(); +beforeAll(() => mockServer.startServer()); afterEach(() => mockServer.unloadScenario()); afterAll(() => mockServer.stopServer()); diff --git a/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-submission-integration.test.ts b/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-submission-integration.test.ts index a6e4ce1d0..458b3d7bd 100644 --- a/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-submission-integration.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-submission-integration.test.ts @@ -16,6 +16,7 @@ import { createVSCodeCommandManager } from "../../../../src/common/vscode/comman import { AllCommands } from "../../../../src/common/commands"; const mockServer = new MockGitHubApiServer(); +beforeAll(() => mockServer.startServer()); afterEach(() => mockServer.unloadScenario()); afterAll(() => mockServer.stopServer());