Merge pull request #1804 from github/koesie10/stop-monitor-after-remove

Stop variant analysis monitor after removing the variant analysis from the query history

Author: koesie10

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

@@ -94,7 +94,12 @@ export class VariantAnalysisManager
     private readonly variantAnalysisResultsManager: VariantAnalysisResultsManager,
   ) {
     super();
-    this.variantAnalysisMonitor = this.push(new VariantAnalysisMonitor(ctx));
+    this.variantAnalysisMonitor = this.push(
+      new VariantAnalysisMonitor(
+        ctx,
+        this.shouldCancelMonitorVariantAnalysis.bind(this),
+      ),
+    );
     this.variantAnalysisMonitor.onVariantAnalysisChange(
       this.onVariantAnalysisUpdated.bind(this),
     );
@@ -319,13 +324,23 @@ export class VariantAnalysisManager
     return await fs.pathExists(filePath);
   }
 
+  private async shouldCancelMonitorVariantAnalysis(
+    variantAnalysisId: number,
+  ): Promise<boolean> {
+    return !this.variantAnalyses.has(variantAnalysisId);
+  }
+
   public async onVariantAnalysisUpdated(
     variantAnalysis: VariantAnalysis | undefined,
   ): Promise<void> {
     if (!variantAnalysis) {
       return;
     }
 
+    if (!this.variantAnalyses.has(variantAnalysis.id)) {
+      return;
+    }
+
     await this.setVariantAnalysis(variantAnalysis);
     this._onVariantAnalysisStatusUpdated.fire(variantAnalysis);
   }

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

@@ -28,7 +28,12 @@ export class VariantAnalysisMonitor extends DisposableObject {
   );
   readonly onVariantAnalysisChange = this._onVariantAnalysisChange.event;
 
-  constructor(private readonly extensionContext: ExtensionContext) {
+  constructor(
+    private readonly extensionContext: ExtensionContext,
+    private readonly shouldCancelMonitor: (
+      variantAnalysisId: number,
+    ) => Promise<boolean>,
+  ) {
     super();
   }
 
@@ -51,6 +56,10 @@ export class VariantAnalysisMonitor extends DisposableObject {
         return;
       }
 
+      if (await this.shouldCancelMonitor(variantAnalysis.id)) {
+        return;
+      }
+
       const variantAnalysisSummary = await ghApiClient.getVariantAnalysis(
         credentials,
         variantAnalysis.controllerRepo.id,

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

@@ -748,9 +748,13 @@ describe("Variant Analysis Manager", () => {
         });
 
         it("should remove variant analysis", async () => {
-          await variantAnalysisManager.onVariantAnalysisUpdated(
+          pathExistsStub.mockImplementation(() => true);
+          await variantAnalysisManager.rehydrateVariantAnalysis(
             dummyVariantAnalysis,
           );
+          expect(pathExistsStub).toBeCalledWith(
+            path.join(storagePath, dummyVariantAnalysis.id.toString()),
+          );
           expect(variantAnalysisManager.variantAnalysesSize).toBe(1);
 
           await variantAnalysisManager.removeVariantAnalysis(

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

@@ -36,6 +36,7 @@ describe("Variant Analysis Monitor", () => {
   >;
   let cancellationTokenSource: CancellationTokenSource;
   let variantAnalysisMonitor: VariantAnalysisMonitor;
+  let shouldCancelMonitor: jest.Mock<Promise<boolean>, [number]>;
   let variantAnalysis: VariantAnalysis;
   let variantAnalysisManager: VariantAnalysisManager;
   let mockGetDownloadResult: jest.SpiedFunction<
@@ -53,12 +54,17 @@ describe("Variant Analysis Monitor", () => {
 
     variantAnalysis = createMockVariantAnalysis({});
 
+    shouldCancelMonitor = jest.fn();
+
     extension = await extensions
       .getExtension<CodeQLExtensionInterface | Record<string, never>>(
         "GitHub.vscode-codeql",
       )!
       .activate();
-    variantAnalysisMonitor = new VariantAnalysisMonitor(extension.ctx);
+    variantAnalysisMonitor = new VariantAnalysisMonitor(
+      extension.ctx,
+      shouldCancelMonitor,
+    );
     variantAnalysisMonitor.onVariantAnalysisChange(onVariantAnalysisChangeSpy);
 
     variantAnalysisManager = extension.variantAnalysisManager;
@@ -114,6 +120,17 @@ describe("Variant Analysis Monitor", () => {
       expect(onVariantAnalysisChangeSpy).not.toHaveBeenCalled();
     });
 
+    it("should return early if variant analysis should be cancelled", async () => {
+      shouldCancelMonitor.mockResolvedValue(true);
+
+      await variantAnalysisMonitor.monitorVariantAnalysis(
+        variantAnalysis,
+        cancellationTokenSource.token,
+      );
+
+      expect(onVariantAnalysisChangeSpy).not.toHaveBeenCalled();
+    });
+
     describe("when the variant analysis fails", () => {
       let mockFailedApiResponse: VariantAnalysisApiResponse;