From 43de90f03d832ecae49906c7a1ca8d38970cad5e Mon Sep 17 00:00:00 2001 From: shati-patel <42641846+shati-patel@users.noreply.github.com> Date: Wed, 19 Oct 2022 15:25:11 +0100 Subject: [PATCH] Pass `variantAnalysisStorageLocation` to the results manager --- .../variant-analysis-manager.ts | 15 ++++++---- .../variant-analysis-results-manager.ts | 30 ++++++++----------- .../variant-analysis-results-manager.test.ts | 21 +++++++++---- 3 files changed, 38 insertions(+), 28 deletions(-) diff --git a/extensions/ql-vscode/src/remote-queries/variant-analysis-manager.ts b/extensions/ql-vscode/src/remote-queries/variant-analysis-manager.ts index 64653128d..4587e4b13 100644 --- a/extensions/ql-vscode/src/remote-queries/variant-analysis-manager.ts +++ b/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 { @@ -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}` + ); } /** diff --git a/extensions/ql-vscode/src/remote-queries/variant-analysis-results-manager.ts b/extensions/ql-vscode/src/remote-queries/variant-analysis-results-manager.ts index baaa7885c..346c0af16 100644 --- a/extensions/ql-vscode/src/remote-queries/variant-analysis-results-manager.ts +++ b/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 { 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,18 +82,20 @@ export class VariantAnalysisResultsManager extends DisposableObject { public async loadResults( variantAnalysisId: number, + variantAnalysisStoragePath: string, repositoryFullName: string ): Promise { 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 { - 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; @@ -101,13 +103,14 @@ export class VariantAnalysisResultsManager extends DisposableObject { private async loadResultsFromStorage( variantAnalysisId: number, + variantAnalysisStoragePath: string, repositoryFullName: string, ): Promise { - 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 { - 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 { @@ -165,16 +168,9 @@ export class VariantAnalysisResultsManager extends DisposableObject { return processedSarif.alerts; } - public getStorageDirectory(variantAnalysisId: number): string { + public getRepoStorageDirectory(variantAnalysisStoragePath: string, fullName: string): string { return path.join( - this.storagePath, - `${variantAnalysisId}` - ); - } - - public getRepoStorageDirectory(variantAnalysisId: number, fullName: string): string { - return path.join( - this.getStorageDirectory(variantAnalysisId), + variantAnalysisStoragePath, fullName ); } diff --git a/extensions/ql-vscode/src/vscode-tests/cli-integration/remote-queries/variant-analysis-results-manager.test.ts b/extensions/ql-vscode/src/vscode-tests/cli-integration/remote-queries/variant-analysis-results-manager.test.ts index b7d584b0a..948ab0869 100644 --- a/extensions/ql-vscode/src/vscode-tests/cli-integration/remote-queries/variant-analysis-results-manager.test.ts +++ b/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>('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;