Skip to content

Commit ff3b609

Browse files
committed
Stop variant analysis monitor after removing it
This will stop the variant analysis monitor from monitoring when a variant analysis is removed from the query history. Since the variant analysis monitor cannot depend on the variant analysis manager (this would create a circular dependency), a function is passed into the variant analysis monitor for checking whether the variant analysis should be cancelled. This commit will also ensure that even if a variant analysis comes in through the `onVariantAnalysisChange` callback, it won't be added to the variant analysis map of the manager.
1 parent a1b81d9 commit ff3b609

3 files changed

Lines changed: 43 additions & 3 deletions

File tree

extensions/ql-vscode/src/remote-queries/variant-analysis-manager.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,12 @@ export class VariantAnalysisManager
9494
private readonly variantAnalysisResultsManager: VariantAnalysisResultsManager,
9595
) {
9696
super();
97-
this.variantAnalysisMonitor = this.push(new VariantAnalysisMonitor(ctx));
97+
this.variantAnalysisMonitor = this.push(
98+
new VariantAnalysisMonitor(
99+
ctx,
100+
this.shouldCancelMonitorVariantAnalysis.bind(this),
101+
),
102+
);
98103
this.variantAnalysisMonitor.onVariantAnalysisChange(
99104
this.onVariantAnalysisUpdated.bind(this),
100105
);
@@ -319,13 +324,23 @@ export class VariantAnalysisManager
319324
return await fs.pathExists(filePath);
320325
}
321326

327+
private async shouldCancelMonitorVariantAnalysis(
328+
variantAnalysisId: number,
329+
): Promise<boolean> {
330+
return !this.variantAnalyses.has(variantAnalysisId);
331+
}
332+
322333
public async onVariantAnalysisUpdated(
323334
variantAnalysis: VariantAnalysis | undefined,
324335
): Promise<void> {
325336
if (!variantAnalysis) {
326337
return;
327338
}
328339

340+
if (!this.variantAnalyses.has(variantAnalysis.id)) {
341+
return;
342+
}
343+
329344
await this.setVariantAnalysis(variantAnalysis);
330345
this._onVariantAnalysisStatusUpdated.fire(variantAnalysis);
331346
}

extensions/ql-vscode/src/remote-queries/variant-analysis-monitor.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,12 @@ export class VariantAnalysisMonitor extends DisposableObject {
2929
);
3030
readonly onVariantAnalysisChange = this._onVariantAnalysisChange.event;
3131

32-
constructor(private readonly extensionContext: ExtensionContext) {
32+
constructor(
33+
private readonly extensionContext: ExtensionContext,
34+
private readonly shouldCancelMonitor: (
35+
variantAnalysisId: number,
36+
) => Promise<boolean>,
37+
) {
3338
super();
3439
}
3540

@@ -52,6 +57,10 @@ export class VariantAnalysisMonitor extends DisposableObject {
5257
return { status: "Canceled" };
5358
}
5459

60+
if (await this.shouldCancelMonitor(variantAnalysis.id)) {
61+
return { status: "Canceled" };
62+
}
63+
5564
const variantAnalysisSummary = await ghApiClient.getVariantAnalysis(
5665
credentials,
5766
variantAnalysis.controllerRepo.id,

extensions/ql-vscode/src/vscode-tests/cli-integration/remote-queries/variant-analysis-monitor.test.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,13 +37,15 @@ describe("Variant Analysis Monitor", async function () {
3737
let mockGetVariantAnalysis: sinon.SinonStub;
3838
let cancellationTokenSource: CancellationTokenSource;
3939
let variantAnalysisMonitor: VariantAnalysisMonitor;
40+
let shouldCancelMonitor: sinon.SinonStub;
4041
let variantAnalysis: VariantAnalysis;
4142
let variantAnalysisManager: VariantAnalysisManager;
4243
let mockGetDownloadResult: sinon.SinonStub;
4344

4445
beforeEach(async () => {
4546
sandbox = sinon.createSandbox();
4647
sandbox.stub(config, "isVariantAnalysisLiveResultsEnabled").returns(false);
48+
shouldCancelMonitor = sinon.stub();
4749

4850
cancellationTokenSource = new CancellationTokenSource();
4951

@@ -55,7 +57,10 @@ describe("Variant Analysis Monitor", async function () {
5557
"GitHub.vscode-codeql",
5658
)!
5759
.activate();
58-
variantAnalysisMonitor = new VariantAnalysisMonitor(extension.ctx);
60+
variantAnalysisMonitor = new VariantAnalysisMonitor(
61+
extension.ctx,
62+
shouldCancelMonitor,
63+
);
5964
} catch (e) {
6065
fail(e as Error);
6166
}
@@ -112,6 +117,17 @@ describe("Variant Analysis Monitor", async function () {
112117
expect(result).to.eql({ status: "Canceled" });
113118
});
114119

120+
it("should return early if variant analysis should be cancelled", async () => {
121+
shouldCancelMonitor.resolves(true);
122+
123+
const result = await variantAnalysisMonitor.monitorVariantAnalysis(
124+
variantAnalysis,
125+
cancellationTokenSource.token,
126+
);
127+
128+
expect(result).to.eql({ status: "Canceled" });
129+
});
130+
115131
describe("when the variant analysis fails", async () => {
116132
let mockFailedApiResponse: VariantAnalysisApiResponse;
117133

0 commit comments

Comments
 (0)