From f18f1b0ca7f803c1d74664036693f5236b4d11bf Mon Sep 17 00:00:00 2001 From: Elena Tanasoiu Date: Tue, 25 Oct 2022 16:02:37 +0100 Subject: [PATCH] Implement `handleRemoveHistoryItem` for variant analysis history items We had previously added a no-op placeholder for when we attempt to remove a variant analysis from our query history. This adds the implementation: - removes the item from the query history - cleans up any existing result files attached to the variant analysis NB: The remote queries would store all their results in a single folder. For variant analysis, we store results per repo. The folder names are build using a cache key and are stored in `cachedResults`. The cache key is built from the variant analysis id and the repo name. In order to delete the results, we've had to pass in the full variant analysis object to the manager and call `cacheResults.delete()` for each of its scanned repos. Co-authored-by: Charis Kyriakou Co-authored-by: Nora Scheuch --- extensions/ql-vscode/src/query-history.ts | 16 ++++++++++++++-- .../remote-queries/variant-analysis-manager.ts | 11 +++++++++++ .../variant-analysis-results-manager.ts | 15 ++++++++++++++- 3 files changed, 39 insertions(+), 3 deletions(-) diff --git a/extensions/ql-vscode/src/query-history.ts b/extensions/ql-vscode/src/query-history.ts index 78eb0e027..3b0692f59 100644 --- a/extensions/ql-vscode/src/query-history.ts +++ b/extensions/ql-vscode/src/query-history.ts @@ -635,7 +635,7 @@ export class QueryHistoryManager extends DisposableObject { const variantAnalysisRemovedSubscription = this.variantAnalysisManager.onVariantAnalysisRemoved(async (variantAnalysis) => { const items = this.treeDataProvider.allHistory.filter(i => i.t === 'variant-analysis' && i.variantAnalysis.id === variantAnalysis.id); items.forEach(async (item) => { - await this.removeRemoteQuery(item as RemoteQueryHistoryItem); + await this.removeVariantAnalysis(item as VariantAnalysisHistoryItem); }); }); @@ -786,7 +786,7 @@ export class QueryHistoryManager extends DisposableObject { } else if (item.t === 'remote') { await this.removeRemoteQuery(item); } else if (item.t === 'variant-analysis') { - // TODO + await this.removeVariantAnalysis(item); } else { assertNever(item); } @@ -812,6 +812,18 @@ export class QueryHistoryManager extends DisposableObject { await this.remoteQueriesManager.removeRemoteQuery(item.queryId); } + private async removeVariantAnalysis(item: VariantAnalysisHistoryItem): Promise { + // Remote queries can be removed locally, but not remotely. + // The user must cancel the query on GitHub Actions explicitly. + this.treeDataProvider.remove(item); + void logger.log(`Deleted ${this.labelProvider.getLabel(item)}.`); + if (item.status === QueryStatus.InProgress) { + void logger.log('The variant analysis is still running on GitHub Actions. To cancel there, you must go to the workflow run in your browser.'); + } + + await this.variantAnalysisManager.removeVariantAnalysis(item.variantAnalysis); + } + async handleSortByName() { if (this.treeDataProvider.sortOrder === SortOrder.NameAsc) { this.treeDataProvider.sortOrder = SortOrder.NameDesc; 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 80921270f..c7321fa5f 100644 --- a/extensions/ql-vscode/src/remote-queries/variant-analysis-manager.ts +++ b/extensions/ql-vscode/src/remote-queries/variant-analysis-manager.ts @@ -72,6 +72,17 @@ export class VariantAnalysisManager extends DisposableObject implements VariantA } } + public async removeVariantAnalysis(variantAnalysis: VariantAnalysis) { + this.variantAnalysisResultsManager.removeAnalysesResults(variantAnalysis); + await this.removeStorageDirectory(variantAnalysis.id); + this.variantAnalyses.delete(variantAnalysis.id); + } + + private async removeStorageDirectory(variantAnalysisId: number) { + const storageLocation = this.getVariantAnalysisStorageLocation(variantAnalysisId); + await fs.remove(storageLocation); + } + public async showView(variantAnalysisId: number): Promise { if (!this.variantAnalyses.get(variantAnalysisId)) { void showAndLogErrorMessage(`No variant analysis found with id: ${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 346c0af16..ab4201f4f 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 @@ -9,7 +9,7 @@ import { sarifParser } from '../sarif-parser'; import { extractAnalysisAlerts } from './sarif-processing'; import { CodeQLCliServer } from '../cli'; import { extractRawResults } from './bqrs-processing'; -import { VariantAnalysisScannedRepositoryResult } from './shared/variant-analysis'; +import { VariantAnalysis, VariantAnalysisScannedRepositoryResult } from './shared/variant-analysis'; import { DisposableObject, DisposeHandler } from '../pure/disposable-object'; import { VariantAnalysisRepoTask } from './gh-api/variant-analysis'; import * as ghApiClient from './gh-api/gh-api-client'; @@ -179,6 +179,19 @@ export class VariantAnalysisResultsManager extends DisposableObject { return `https://github.com/${fullName}/blob/${sha}`; } + public removeAnalysesResults(variantAnalysis: VariantAnalysis) { + const scannedRepos = variantAnalysis.scannedRepos; + + if (scannedRepos) { + scannedRepos.forEach(scannedRepo => { + const cacheKey = createCacheKey(variantAnalysis.id, scannedRepo.repository.fullName); + if (this.cachedResults.get(cacheKey)) { + this.cachedResults.delete(cacheKey); + } + }); + } + } + public dispose(disposeHandler?: DisposeHandler) { super.dispose(disposeHandler);