Pass variantAnalysisStorageLocation to the results manager

Author: shati-patel

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

@@ -1,3 +1,5 @@
+import * as path from 'path';
+
 import * as ghApiClient from './gh-api/gh-api-client';
 import { CancellationToken, commands, EventEmitter, ExtensionContext, window } from 'vscode';
 import { DisposableObject } from '../pure/disposable-object';
@@ -39,14 +41,14 @@ export class VariantAnalysisManager extends DisposableObject implements VariantA
   constructor(
     private readonly ctx: ExtensionContext,
     cliServer: CodeQLCliServer,
-    storagePath: string,
+    private readonly storagePath: string,
     logger: Logger,
   ) {
     super();
     this.variantAnalysisMonitor = this.push(new VariantAnalysisMonitor(ctx));
     this.variantAnalysisMonitor.onVariantAnalysisChange(this.onVariantAnalysisUpdated.bind(this));
 
-    this.variantAnalysisResultsManager = this.push(new VariantAnalysisResultsManager(cliServer, storagePath, logger));
+    this.variantAnalysisResultsManager = this.push(new VariantAnalysisResultsManager(cliServer, logger));
     this.variantAnalysisResultsManager.onResultLoaded(this.onRepoResultLoaded.bind(this));
   }
 
@@ -87,7 +89,7 @@ export class VariantAnalysisManager extends DisposableObject implements VariantA
       throw new Error(`No variant analysis with id: ${variantAnalysisId}`);
     }
 
-    await this.variantAnalysisResultsManager.loadResults(variantAnalysisId, repositoryFullName);
+    await this.variantAnalysisResultsManager.loadResults(variantAnalysisId, this.getVariantAnalysisStorageLocation(variantAnalysisId), repositoryFullName);
   }
 
   private async onVariantAnalysisUpdated(variantAnalysis: VariantAnalysis | undefined): Promise<void> {
@@ -160,7 +162,7 @@ export class VariantAnalysisManager extends DisposableObject implements VariantA
       repoState.downloadStatus = VariantAnalysisScannedRepositoryDownloadStatus.InProgress;
       await this.onRepoStateUpdated(variantAnalysisSummary.id, repoState);
 
-      await this.variantAnalysisResultsManager.download(credentials, variantAnalysisSummary.id, repoTask);
+      await this.variantAnalysisResultsManager.download(credentials, variantAnalysisSummary.id, repoTask, this.getVariantAnalysisStorageLocation(variantAnalysisSummary.id));
     }
 
     repoState.downloadStatus = VariantAnalysisScannedRepositoryDownloadStatus.Succeeded;
@@ -180,7 +182,10 @@ export class VariantAnalysisManager extends DisposableObject implements VariantA
   }
 
   public getVariantAnalysisStorageLocation(variantAnalysisId: number): string {
-    return this.variantAnalysisResultsManager.getStorageDirectory(variantAnalysisId);
+    return path.join(
+      this.storagePath,
+      `${variantAnalysisId}`
+    );
   }
 
   /**

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

@@ -39,7 +39,6 @@ export class VariantAnalysisResultsManager extends DisposableObject {
 
   constructor(
     private readonly cliServer: CodeQLCliServer,
-    private readonly storagePath: string,
     private readonly logger: Logger,
   ) {
     super();
@@ -50,12 +49,13 @@ export class VariantAnalysisResultsManager extends DisposableObject {
     credentials: Credentials,
     variantAnalysisId: number,
     repoTask: VariantAnalysisRepoTask,
+    variantAnalysisStoragePath: string,
   ): Promise<void> {
     if (!repoTask.artifact_url) {
       throw new Error('Missing artifact URL');
     }
 
-    const resultDirectory = this.getRepoStorageDirectory(variantAnalysisId, repoTask.repository.full_name);
+    const resultDirectory = this.getRepoStorageDirectory(variantAnalysisStoragePath, repoTask.repository.full_name);
 
     const result = await ghApiClient.getVariantAnalysisRepoResult(
       credentials,
@@ -82,32 +82,35 @@ export class VariantAnalysisResultsManager extends DisposableObject {
 
   public async loadResults(
     variantAnalysisId: number,
+    variantAnalysisStoragePath: string,
     repositoryFullName: string
   ): Promise<VariantAnalysisScannedRepositoryResult> {
     const result = this.cachedResults.get(createCacheKey(variantAnalysisId, repositoryFullName));
 
-    return result ?? await this.loadResultsIntoMemory(variantAnalysisId, repositoryFullName);
+    return result ?? await this.loadResultsIntoMemory(variantAnalysisId, variantAnalysisStoragePath, repositoryFullName);
   }
 
   private async loadResultsIntoMemory(
     variantAnalysisId: number,
+    variantAnalysisStoragePath: string,
     repositoryFullName: string,
   ): Promise<VariantAnalysisScannedRepositoryResult> {
-    const result = await this.loadResultsFromStorage(variantAnalysisId, repositoryFullName);
+    const result = await this.loadResultsFromStorage(variantAnalysisId, variantAnalysisStoragePath, repositoryFullName);
     this.cachedResults.set(createCacheKey(variantAnalysisId, repositoryFullName), result);
     this._onResultLoaded.fire(result);
     return result;
   }
 
   private async loadResultsFromStorage(
     variantAnalysisId: number,
+    variantAnalysisStoragePath: string,
     repositoryFullName: string,
   ): Promise<VariantAnalysisScannedRepositoryResult> {
-    if (!(await this.isVariantAnalysisRepoDownloaded(variantAnalysisId, repositoryFullName))) {
+    if (!(await this.isVariantAnalysisRepoDownloaded(variantAnalysisStoragePath, repositoryFullName))) {
       throw new Error('Variant analysis results not downloaded');
     }
 
-    const storageDirectory = this.getRepoStorageDirectory(variantAnalysisId, repositoryFullName);
+    const storageDirectory = this.getRepoStorageDirectory(variantAnalysisStoragePath, repositoryFullName);
 
     const repoTask: VariantAnalysisRepoTask = await fs.readJson(path.join(storageDirectory, VariantAnalysisResultsManager.REPO_TASK_FILENAME));
 
@@ -144,10 +147,10 @@ export class VariantAnalysisResultsManager extends DisposableObject {
   }
 
   private async isVariantAnalysisRepoDownloaded(
-    variantAnalysisId: number,
+    variantAnalysisStoragePath: string,
     repositoryFullName: string,
   ): Promise<boolean> {
-    return await fs.pathExists(this.getRepoStorageDirectory(variantAnalysisId, repositoryFullName));
+    return await fs.pathExists(this.getRepoStorageDirectory(variantAnalysisStoragePath, repositoryFullName));
   }
 
   private async readBqrsResults(filePath: string, fileLinkPrefix: string, sourceLocationPrefix: string): Promise<AnalysisRawResults> {
@@ -165,16 +168,9 @@ export class VariantAnalysisResultsManager extends DisposableObject {
     return processedSarif.alerts;
   }
 
-  public getStorageDirectory(variantAnalysisId: number): string {
-    return path.join(
-      this.storagePath,
-      `${variantAnalysisId}`
-    );
-  }
-
-  public getRepoStorageDirectory(variantAnalysisId: number, fullName: string): string {
+  public getRepoStorageDirectory(variantAnalysisStoragePath: string, fullName: string): string {
     return path.join(
-      this.getStorageDirectory(variantAnalysisId),
+      variantAnalysisStoragePath,
       fullName
     );
   }

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

@@ -33,7 +33,7 @@ describe(VariantAnalysisResultsManager.name, () => {
     try {
       const extension = await extensions.getExtension<CodeQLExtensionInterface | Record<string, never>>('GitHub.vscode-codeql')!.activate();
       cli = extension.cliServer;
-      variantAnalysisResultsManager = new VariantAnalysisResultsManager(cli, storagePath, logger);
+      variantAnalysisResultsManager = new VariantAnalysisResultsManager(cli, logger);
     } catch (e) {
       fail(e as Error);
     }
@@ -45,12 +45,17 @@ describe(VariantAnalysisResultsManager.name, () => {
 
   describe('download', () => {
     let getOctokitStub: sinon.SinonStub;
+    let variantAnalysisStoragePath: string;
     const mockCredentials = {
       getOctokit: () => Promise.resolve({
         request: getOctokitStub
       })
     } as unknown as Credentials;
 
+    beforeEach(async () => {
+      variantAnalysisStoragePath = path.join(storagePath, variantAnalysisId.toString());
+    });
+
     describe('when the artifact_url is missing', async () => {
       it('should not try to download the result', async () => {
         const dummyRepoTask = createMockVariantAnalysisRepoTask();
@@ -60,7 +65,8 @@ describe(VariantAnalysisResultsManager.name, () => {
           await variantAnalysisResultsManager.download(
             mockCredentials,
             variantAnalysisId,
-            dummyRepoTask
+            dummyRepoTask,
+            variantAnalysisStoragePath
           );
 
           expect.fail('Expected an error to be thrown');
@@ -78,7 +84,7 @@ describe(VariantAnalysisResultsManager.name, () => {
       beforeEach(async () => {
         dummyRepoTask = createMockVariantAnalysisRepoTask();
 
-        storageDirectory = variantAnalysisResultsManager.getRepoStorageDirectory(variantAnalysisId, dummyRepoTask.repository.full_name);
+        storageDirectory = variantAnalysisResultsManager.getRepoStorageDirectory(variantAnalysisStoragePath, dummyRepoTask.repository.full_name);
         const sourceFilePath = path.join(__dirname, '../../../../src/vscode-tests/cli-integration/data/variant-analysis-results.zip');
         arrayBuffer = fs.readFileSync(sourceFilePath).buffer;
 
@@ -97,7 +103,8 @@ describe(VariantAnalysisResultsManager.name, () => {
         await variantAnalysisResultsManager.download(
           mockCredentials,
           variantAnalysisId,
-          dummyRepoTask
+          dummyRepoTask,
+          variantAnalysisStoragePath
         );
 
         expect(getVariantAnalysisRepoResultStub.calledOnce).to.be.true;
@@ -107,7 +114,8 @@ describe(VariantAnalysisResultsManager.name, () => {
         await variantAnalysisResultsManager.download(
           mockCredentials,
           variantAnalysisId,
-          dummyRepoTask
+          dummyRepoTask,
+          variantAnalysisStoragePath
         );
 
         expect(fs.existsSync(`${storageDirectory}/results.zip`)).to.be.true;
@@ -117,7 +125,8 @@ describe(VariantAnalysisResultsManager.name, () => {
         await variantAnalysisResultsManager.download(
           mockCredentials,
           variantAnalysisId,
-          dummyRepoTask
+          dummyRepoTask,
+          variantAnalysisStoragePath
         );
 
         expect(fs.existsSync(`${storageDirectory}/results/results.sarif`)).to.be.true;