Merge branch 'main' into shati-patel/query-history-get-id

.github/workflows/main.yml

@@ -139,7 +139,7 @@ jobs:
     strategy:
       matrix:
         os: [ubuntu-latest, windows-latest]
-        version: ['v2.6.3', 'v2.7.6', 'v2.8.5', 'v2.9.4', 'v2.10.5', 'v2.11.1', 'nightly']
+        version: ['v2.7.6', 'v2.8.5', 'v2.9.4', 'v2.10.5', 'v2.11.1', 'nightly']
     env:
       CLI_VERSION: ${{ matrix.version }}
       NIGHTLY_URL: ${{ needs.find-nightly.outputs.url }}

extensions/ql-vscode/CHANGELOG.md

@@ -1,5 +1,7 @@
 # CodeQL for Visual Studio Code: Changelog
 
+## [UNRELEASED]
+
 ## 1.7.2 - 14 October 2022
 
 - Fix a bug where results created in older versions were thought to be unsuccessful. [#1605](https://github.com/github/vscode-codeql/pull/1605)

extensions/ql-vscode/package-lock.json

@@ -1,12 +1,12 @@
 {
   "name": "vscode-codeql",
-  "version": "1.7.2",
+  "version": "1.7.3",
   "lockfileVersion": 2,
   "requires": true,
   "packages": {
     "": {
       "name": "vscode-codeql",
-      "version": "1.7.2",
+      "version": "1.7.3",
       "license": "MIT",
       "dependencies": {
         "@octokit/plugin-retry": "^3.0.9",
@@ -26,6 +26,7 @@
         "minimist": "~1.2.6",
         "nanoid": "^3.2.0",
         "node-fetch": "~2.6.7",
+        "p-queue": "^6.0.0",
         "path-browserify": "^1.0.1",
         "react": "^17.0.2",
         "react-dom": "^17.0.2",
@@ -21365,6 +21366,11 @@
         "es5-ext": "~0.10.14"
       }
     },
+    "node_modules/eventemitter3": {
+      "version": "4.0.7",
+      "resolved": "https://registry.npmjs.org/eventemitter3/-/eventemitter3-4.0.7.tgz",
+      "integrity": "sha512-8guHBZCwKnFhYdHr2ysuRWErTwhoN2X8XELRlrRwpmfeY2jjuUN4taQMsULKUVo1K4DvZl+0pgfyoysHxvmvEw=="
+    },
     "node_modules/events": {
       "version": "3.3.0",
       "resolved": "https://registry.npmjs.org/events/-/events-3.3.0.tgz",
@@ -32968,7 +32974,6 @@
       "version": "1.0.0",
       "resolved": "https://registry.npmjs.org/p-finally/-/p-finally-1.0.0.tgz",
       "integrity": "sha512-LICb2p9CB7FS+0eR1oqWnHhp0FljGLZCWBE9aix0Uye9W8LTQPwMTYVGWQWIw9RdQiDg4+epXQODwIYJtSJaow==",
-      "dev": true,
       "engines": {
         "node": ">=4"
       }
@@ -33009,11 +33014,25 @@
         "node": ">=10"
       }
     },
+    "node_modules/p-queue": {
+      "version": "6.6.2",
+      "resolved": "https://registry.npmjs.org/p-queue/-/p-queue-6.6.2.tgz",
+      "integrity": "sha512-RwFpb72c/BhQLEXIZ5K2e+AhgNVmIejGlTgiB9MzZ0e93GRvqZ7uSi0dvRF7/XIXDeNkra2fNHBxTyPDGySpjQ==",
+      "dependencies": {
+        "eventemitter3": "^4.0.4",
+        "p-timeout": "^3.2.0"
+      },
+      "engines": {
+        "node": ">=8"
+      },
+      "funding": {
+        "url": "https://github.com/sponsors/sindresorhus"
+      }
+    },
     "node_modules/p-timeout": {
       "version": "3.2.0",
       "resolved": "https://registry.npmjs.org/p-timeout/-/p-timeout-3.2.0.tgz",
       "integrity": "sha512-rhIwUycgwwKcP9yTOOFK/AKsAopjjCakVqLHePO3CC6Mir1Z99xT+R63jZxAT5lFZLa2inS5h+ZS2GvR99/FBg==",
-      "dev": true,
       "dependencies": {
         "p-finally": "^1.0.0"
       },
@@ -56515,6 +56534,11 @@
         "es5-ext": "~0.10.14"
       }
     },
+    "eventemitter3": {
+      "version": "4.0.7",
+      "resolved": "https://registry.npmjs.org/eventemitter3/-/eventemitter3-4.0.7.tgz",
+      "integrity": "sha512-8guHBZCwKnFhYdHr2ysuRWErTwhoN2X8XELRlrRwpmfeY2jjuUN4taQMsULKUVo1K4DvZl+0pgfyoysHxvmvEw=="
+    },
     "events": {
       "version": "3.3.0",
       "resolved": "https://registry.npmjs.org/events/-/events-3.3.0.tgz",
@@ -65468,8 +65492,7 @@
     "p-finally": {
       "version": "1.0.0",
       "resolved": "https://registry.npmjs.org/p-finally/-/p-finally-1.0.0.tgz",
-      "integrity": "sha512-LICb2p9CB7FS+0eR1oqWnHhp0FljGLZCWBE9aix0Uye9W8LTQPwMTYVGWQWIw9RdQiDg4+epXQODwIYJtSJaow==",
-      "dev": true
+      "integrity": "sha512-LICb2p9CB7FS+0eR1oqWnHhp0FljGLZCWBE9aix0Uye9W8LTQPwMTYVGWQWIw9RdQiDg4+epXQODwIYJtSJaow=="
     },
     "p-limit": {
       "version": "2.3.0",
@@ -65498,11 +65521,19 @@
         "aggregate-error": "^3.0.0"
       }
     },
+    "p-queue": {
+      "version": "6.6.2",
+      "resolved": "https://registry.npmjs.org/p-queue/-/p-queue-6.6.2.tgz",
+      "integrity": "sha512-RwFpb72c/BhQLEXIZ5K2e+AhgNVmIejGlTgiB9MzZ0e93GRvqZ7uSi0dvRF7/XIXDeNkra2fNHBxTyPDGySpjQ==",
+      "requires": {
+        "eventemitter3": "^4.0.4",
+        "p-timeout": "^3.2.0"
+      }
+    },
     "p-timeout": {
       "version": "3.2.0",
       "resolved": "https://registry.npmjs.org/p-timeout/-/p-timeout-3.2.0.tgz",
       "integrity": "sha512-rhIwUycgwwKcP9yTOOFK/AKsAopjjCakVqLHePO3CC6Mir1Z99xT+R63jZxAT5lFZLa2inS5h+ZS2GvR99/FBg==",
-      "dev": true,
       "requires": {
         "p-finally": "^1.0.0"
       }

extensions/ql-vscode/package.json

@@ -4,7 +4,7 @@
   "description": "CodeQL for Visual Studio Code",
   "author": "GitHub",
   "private": true,
-  "version": "1.7.2",
+  "version": "1.7.3",
   "publisher": "GitHub",
   "license": "MIT",
   "icon": "media/VS-marketplace-CodeQL-icon.png",
@@ -1230,6 +1230,7 @@
     "minimist": "~1.2.6",
     "nanoid": "^3.2.0",
     "node-fetch": "~2.6.7",
+    "p-queue": "^6.0.0",
     "path-browserify": "^1.0.1",
     "react": "^17.0.2",
     "react-dom": "^17.0.2",

extensions/ql-vscode/src/extension.ts

@@ -951,7 +951,7 @@ async function activateWithInstalledDistribution(
       variantAnalysisSummary: VariantAnalysisApiResponse,
       token: CancellationToken
     ) => {
-      await variantAnalysisManager.autoDownloadVariantAnalysisResult(scannedRepo, variantAnalysisSummary, token);
+      await variantAnalysisManager.enqueueDownload(scannedRepo, variantAnalysisSummary, token);
     })
   );
 

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

@@ -22,6 +22,7 @@ import { VariantAnalysisResultsManager } from './variant-analysis-results-manage
 import { CodeQLCliServer } from '../cli';
 import { getControllerRepo } from './run-remote-query';
 import { processUpdatedVariantAnalysis } from './variant-analysis-processor';
+import PQueue from 'p-queue';
 
 export class VariantAnalysisManager extends DisposableObject implements VariantAnalysisViewManager<VariantAnalysisView> {
   private readonly _onVariantAnalysisAdded = this.push(new EventEmitter<VariantAnalysis>());
@@ -31,6 +32,8 @@ export class VariantAnalysisManager extends DisposableObject implements VariantA
   private readonly variantAnalysisResultsManager: VariantAnalysisResultsManager;
   private readonly variantAnalyses = new Map<number, VariantAnalysis>();
   private readonly views = new Map<number, VariantAnalysisView>();
+  private static readonly maxConcurrentDownloads = 3;
+  private readonly queue = new PQueue({ concurrency: VariantAnalysisManager.maxConcurrentDownloads });
 
   constructor(
     private readonly ctx: ExtensionContext,
@@ -39,7 +42,7 @@ export class VariantAnalysisManager extends DisposableObject implements VariantA
     logger: Logger,
   ) {
     super();
-    this.variantAnalysisMonitor = this.push(new VariantAnalysisMonitor(ctx, logger));
+    this.variantAnalysisMonitor = this.push(new VariantAnalysisMonitor(ctx));
     this.variantAnalysisMonitor.onVariantAnalysisChange(this.onVariantAnalysisUpdated.bind(this));
 
     this.variantAnalysisResultsManager = this.push(new VariantAnalysisResultsManager(cliServer, storagePath, logger));
@@ -161,6 +164,18 @@ export class VariantAnalysisManager extends DisposableObject implements VariantA
     await this.onRepoStateUpdated(variantAnalysisSummary.id, repoState);
   }
 
+  public async enqueueDownload(
+    scannedRepo: ApiVariantAnalysisScannedRepository,
+    variantAnalysisSummary: VariantAnalysisApiResponse,
+    token: CancellationToken
+  ): Promise<void> {
+    await this.queue.add(() => this.autoDownloadVariantAnalysisResult(scannedRepo, variantAnalysisSummary, token));
+  }
+
+  public downloadsQueueSize(): number {
+    return this.queue.pending;
+  }
+
   public async promptOpenVariantAnalysis() {
     const credentials = await Credentials.initialize(this.ctx);
     if (!credentials) { throw Error('Error authenticating with GitHub'); }

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

@@ -1,11 +1,11 @@
 import { ExtensionContext, CancellationToken, commands, EventEmitter } from 'vscode';
 import { Credentials } from '../authentication';
-import { Logger } from '../logging';
 import * as ghApiClient from './gh-api/gh-api-client';
 
 import { VariantAnalysis, VariantAnalysisStatus } from './shared/variant-analysis';
 import {
-  VariantAnalysis as VariantAnalysisApiResponse
+  VariantAnalysis as VariantAnalysisApiResponse,
+  VariantAnalysisScannedRepository
 } from './gh-api/variant-analysis';
 import { VariantAnalysisMonitorResult } from './shared/variant-analysis-monitor-result';
 import { processFailureReason, processUpdatedVariantAnalysis } from './variant-analysis-processor';
@@ -22,7 +22,6 @@ export class VariantAnalysisMonitor extends DisposableObject {
 
   constructor(
     private readonly extensionContext: ExtensionContext,
-    private readonly logger: Logger
   ) {
     super();
   }
@@ -73,16 +72,8 @@ export class VariantAnalysisMonitor extends DisposableObject {
 
       this._onVariantAnalysisChange.fire(variantAnalysis);
 
-      void this.logger.log('****** Retrieved variant analysis' + JSON.stringify(variantAnalysisSummary));
-
-      if (variantAnalysisSummary.scanned_repositories) {
-        variantAnalysisSummary.scanned_repositories.forEach(scannedRepo => {
-          if (!scannedReposDownloaded.includes(scannedRepo.repository.id) && scannedRepo.analysis_status === 'succeeded') {
-            void commands.executeCommand('codeQL.autoDownloadVariantAnalysisResult', scannedRepo, variantAnalysisSummary);
-            scannedReposDownloaded.push(scannedRepo.repository.id);
-          }
-        });
-      }
+      const downloadedRepos = this.downloadVariantAnalysisResults(variantAnalysisSummary, scannedReposDownloaded);
+      scannedReposDownloaded.push(...downloadedRepos);
 
       if (variantAnalysisSummary.status === 'completed') {
         break;
@@ -94,6 +85,46 @@ export class VariantAnalysisMonitor extends DisposableObject {
     return { status: 'CompletedSuccessfully', scannedReposDownloaded: scannedReposDownloaded };
   }
 
+  private scheduleForDownload(
+    scannedRepo: VariantAnalysisScannedRepository,
+    variantAnalysisSummary: VariantAnalysisApiResponse
+  ) {
+    void commands.executeCommand('codeQL.autoDownloadVariantAnalysisResult', scannedRepo, variantAnalysisSummary);
+  }
+
+  private shouldDownload(
+    scannedRepo: VariantAnalysisScannedRepository,
+    alreadyDownloaded: number[]
+  ): boolean {
+    return !alreadyDownloaded.includes(scannedRepo.repository.id) && scannedRepo.analysis_status === 'succeeded';
+  }
+
+  private getReposToDownload(
+    variantAnalysisSummary: VariantAnalysisApiResponse,
+    alreadyDownloaded: number[]
+  ): VariantAnalysisScannedRepository[] {
+    if (variantAnalysisSummary.scanned_repositories) {
+      return variantAnalysisSummary.scanned_repositories.filter(scannedRepo => this.shouldDownload(scannedRepo, alreadyDownloaded));
+    } else {
+      return [];
+    }
+  }
+
+  private downloadVariantAnalysisResults(
+    variantAnalysisSummary: VariantAnalysisApiResponse,
+    scannedReposDownloaded: number[]
+  ): number[] {
+    const repoResultsToDownload = this.getReposToDownload(variantAnalysisSummary, scannedReposDownloaded);
+    const downloadedRepos: number[] = [];
+
+    repoResultsToDownload.forEach(scannedRepo => {
+      downloadedRepos.push(scannedRepo.repository.id);
+      this.scheduleForDownload(scannedRepo, variantAnalysisSummary);
+    });
+
+    return downloadedRepos;
+  }
+
   private async sleep(ms: number) {
     return new Promise(resolve => setTimeout(resolve, ms));
   }

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

@@ -149,6 +149,17 @@ describe('Variant Analysis Manager', async function() {
 
         expect(getVariantAnalysisRepoResultStub.calledOnce).to.be.true;
       });
+
+      it('should pop download tasks off the queue', async () => {
+        const getResultsSpy = sandbox.spy(variantAnalysisManager, 'autoDownloadVariantAnalysisResult');
+
+        await variantAnalysisManager.enqueueDownload(scannedRepos[0], variantAnalysis, cancellationTokenSource.token);
+        await variantAnalysisManager.enqueueDownload(scannedRepos[1], variantAnalysis, cancellationTokenSource.token);
+        await variantAnalysisManager.enqueueDownload(scannedRepos[2], variantAnalysis, cancellationTokenSource.token);
+
+        expect(variantAnalysisManager.downloadsQueueSize()).to.equal(0);
+        expect(getResultsSpy).to.have.been.calledThrice;
+      });
     });
   });
 });

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

@@ -1,16 +1,15 @@
 import * as sinon from 'sinon';
 import { expect } from 'chai';
-import { CancellationTokenSource, extensions } from 'vscode';
+import { CancellationTokenSource, commands, extensions } from 'vscode';
 import { CodeQLExtensionInterface } from '../../../extension';
-import { logger } from '../../../logging';
 import * as config from '../../../config';
 
 import * as ghApiClient from '../../../remote-queries/gh-api/gh-api-client';
 import { VariantAnalysisMonitor } from '../../../remote-queries/variant-analysis-monitor';
 import {
   VariantAnalysis as VariantAnalysisApiResponse,
+  VariantAnalysisFailureReason,
   VariantAnalysisScannedRepository as ApiVariantAnalysisScannedRepository,
-  VariantAnalysisFailureReason
 } from '../../../remote-queries/gh-api/variant-analysis';
 import { createFailedMockApiResponse, createMockApiResponse } from '../../factories/remote-queries/gh-api/variant-analysis-api-response';
 import { VariantAnalysis, VariantAnalysisStatus } from '../../../remote-queries/shared/variant-analysis';
@@ -18,19 +17,22 @@ import { createMockScannedRepos } from '../../factories/remote-queries/gh-api/sc
 import { processFailureReason } from '../../../remote-queries/variant-analysis-processor';
 import { Credentials } from '../../../authentication';
 import { createMockVariantAnalysis } from '../../factories/remote-queries/shared/variant-analysis';
+import { VariantAnalysisManager } from '../../../remote-queries/variant-analysis-manager';
 
 describe('Variant Analysis Monitor', async function() {
   this.timeout(60000);
 
   let sandbox: sinon.SinonSandbox;
+  let extension: CodeQLExtensionInterface | Record<string, never>;
   let mockGetVariantAnalysis: sinon.SinonStub;
   let cancellationTokenSource: CancellationTokenSource;
   let variantAnalysisMonitor: VariantAnalysisMonitor;
   let variantAnalysis: VariantAnalysis;
+  let variantAnalysisManager: VariantAnalysisManager;
+  let mockGetDownloadResult: sinon.SinonStub;
 
   beforeEach(async () => {
     sandbox = sinon.createSandbox();
-    sandbox.stub(logger, 'log');
     sandbox.stub(config, 'isVariantAnalysisLiveResultsEnabled').returns(false);
 
     cancellationTokenSource = new CancellationTokenSource();
@@ -38,12 +40,15 @@ describe('Variant Analysis Monitor', async function() {
     variantAnalysis = createMockVariantAnalysis();
 
     try {
-      const extension = await extensions.getExtension<CodeQLExtensionInterface | Record<string, never>>('GitHub.vscode-codeql')!.activate();
-      variantAnalysisMonitor = new VariantAnalysisMonitor(extension.ctx, logger);
+      extension = await extensions.getExtension<CodeQLExtensionInterface | Record<string, never>>('GitHub.vscode-codeql')!.activate();
+      variantAnalysisMonitor = new VariantAnalysisMonitor(extension.ctx);
     } catch (e) {
       fail(e as Error);
     }
 
+    variantAnalysisManager = extension.variantAnalysisManager;
+    mockGetDownloadResult = sandbox.stub(variantAnalysisManager, 'autoDownloadVariantAnalysisResult');
+
     limitNumberOfAttemptsToMonitor();
   });
 
@@ -112,20 +117,47 @@ describe('Variant Analysis Monitor', async function() {
     describe('when the variant analysis is in progress', async () => {
       let mockApiResponse: VariantAnalysisApiResponse;
       let scannedRepos: ApiVariantAnalysisScannedRepository[];
+      let succeededRepos: ApiVariantAnalysisScannedRepository[];
 
       describe('when there are successfully scanned repos', async () => {
         beforeEach(async function() {
-          scannedRepos = createMockScannedRepos(['pending', 'in_progress', 'succeeded']);
+          scannedRepos = createMockScannedRepos(['pending', 'pending', 'in_progress', 'in_progress', 'succeeded', 'succeeded', 'succeeded']);
           mockApiResponse = createMockApiResponse('completed', scannedRepos);
           mockGetVariantAnalysis = sandbox.stub(ghApiClient, 'getVariantAnalysis').resolves(mockApiResponse);
+          succeededRepos = scannedRepos.filter(r => r.analysis_status === 'succeeded');
         });
 
         it('should succeed and return a list of scanned repo ids', async () => {
           const result = await variantAnalysisMonitor.monitorVariantAnalysis(variantAnalysis, cancellationTokenSource.token);
-          const scannedRepoIds = scannedRepos.filter(r => r.analysis_status == 'succeeded').map(r => r.repository.id);
 
           expect(result.status).to.equal('CompletedSuccessfully');
-          expect(result.scannedReposDownloaded).to.eql(scannedRepoIds);
+          expect(result.scannedReposDownloaded).to.eql(succeededRepos.map(r => r.repository.id));
+        });
+
+        it('should trigger a download extension command for each repo', async () => {
+          const succeededRepos = scannedRepos.filter(r => r.analysis_status === 'succeeded');
+          const commandSpy = sandbox.spy(commands, 'executeCommand');
+
+          await variantAnalysisMonitor.monitorVariantAnalysis(variantAnalysis, cancellationTokenSource.token);
+
+          expect(commandSpy).to.have.callCount(succeededRepos.length);
+
+          succeededRepos.forEach((succeededRepo, index) => {
+            expect(commandSpy.getCall(index).args[0]).to.eq('codeQL.autoDownloadVariantAnalysisResult');
+            expect(commandSpy.getCall(index).args[1]).to.eq(succeededRepo);
+            expect(commandSpy.getCall(index).args[2]).to.eq(mockApiResponse);
+          });
+        });
+
+        it('should download all available results', async () => {
+          await variantAnalysisMonitor.monitorVariantAnalysis(variantAnalysis, cancellationTokenSource.token);
+
+          expect(mockGetDownloadResult).to.have.callCount(succeededRepos.length);
+
+          succeededRepos.forEach((succeededRepo, index) => {
+            expect(mockGetDownloadResult.getCall(index).args[0]).to.eq(succeededRepo);
+            expect(mockGetDownloadResult.getCall(index).args[1]).to.eq(mockApiResponse);
+          });
         });
       });
 
@@ -144,6 +176,12 @@ describe('Variant Analysis Monitor', async function() {
           expect(result.status).to.equal('CompletedSuccessfully');
           expect(result.scannedReposDownloaded).to.eql([]);
         });
+
+        it('should not try to download any repos', async () => {
+          await variantAnalysisMonitor.monitorVariantAnalysis(variantAnalysis, cancellationTokenSource.token);
+
+          expect(mockGetDownloadResult).to.not.have.been.called;
+        });
       });
 
       describe('when there are no repos to scan', async () => {
@@ -159,6 +197,12 @@ describe('Variant Analysis Monitor', async function() {
           expect(result.status).to.equal('CompletedSuccessfully');
           expect(result.scannedReposDownloaded).to.eql([]);
         });
+
+        it('should not try to download any repos', async () => {
+          await variantAnalysisMonitor.monitorVariantAnalysis(variantAnalysis, cancellationTokenSource.token);
+
+          expect(mockGetDownloadResult).to.not.have.been.called;
+        });
       });
     });
   });