diff --git a/extensions/ql-vscode/src/extension.ts b/extensions/ql-vscode/src/extension.ts index c5d16765a..5cfff34fe 100644 --- a/extensions/ql-vscode/src/extension.ts +++ b/extensions/ql-vscode/src/extension.ts @@ -956,6 +956,12 @@ async function activateWithInstalledDistribution( }) ); + ctx.subscriptions.push( + commandRunner('codeQL.loadVariantAnalysisRepoResults', async (variantAnalysisId: number, repositoryFullName: string) => { + await variantAnalysisManager.loadResults(variantAnalysisId, repositoryFullName); + }) + ); + // The "openVariantAnalysisView" command is internal-only. ctx.subscriptions.push( commandRunner('codeQL.openVariantAnalysisView', async (variantAnalysisId: number) => { diff --git a/extensions/ql-vscode/src/pure/interface-types.ts b/extensions/ql-vscode/src/pure/interface-types.ts index c63687260..03033dc8a 100644 --- a/extensions/ql-vscode/src/pure/interface-types.ts +++ b/extensions/ql-vscode/src/pure/interface-types.ts @@ -459,6 +459,11 @@ export interface SetRepoStatesMessage { repoStates: VariantAnalysisScannedRepositoryState[]; } +export interface RequestRepositoryResultsMessage { + t: 'requestRepositoryResults'; + repositoryFullName: string; +} + export type ToVariantAnalysisMessage = | SetVariantAnalysisMessage | SetRepoResultsMessage @@ -466,4 +471,5 @@ export type ToVariantAnalysisMessage = export type FromVariantAnalysisMessage = | ViewLoadedMsg - | StopVariantAnalysisMessage; + | StopVariantAnalysisMessage + | RequestRepositoryResultsMessage; 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 50805f79c..97c96877f 100644 --- a/extensions/ql-vscode/src/remote-queries/variant-analysis-manager.ts +++ b/extensions/ql-vscode/src/remote-queries/variant-analysis-manager.ts @@ -12,6 +12,7 @@ import { import { VariantAnalysis, VariantAnalysisScannedRepositoryDownloadStatus, + VariantAnalysisScannedRepositoryResult, VariantAnalysisScannedRepositoryState } from './shared/variant-analysis'; import { getErrorMessage } from '../pure/helpers-pure'; @@ -26,6 +27,7 @@ export class VariantAnalysisManager extends DisposableObject implements VariantA private readonly variantAnalysisMonitor: VariantAnalysisMonitor; private readonly variantAnalysisResultsManager: VariantAnalysisResultsManager; + private readonly variantAnalyses = new Map(); private readonly views = new Map(); constructor( @@ -39,6 +41,7 @@ export class VariantAnalysisManager extends DisposableObject implements VariantA this.variantAnalysisMonitor.onVariantAnalysisChange(this.onVariantAnalysisUpdated.bind(this)); this.variantAnalysisResultsManager = this.push(new VariantAnalysisResultsManager(cliServer, storagePath, logger)); + this.variantAnalysisResultsManager.onResultLoaded(this.onRepoResultLoaded.bind(this)); } public async showView(variantAnalysisId: number): Promise { @@ -68,11 +71,22 @@ export class VariantAnalysisManager extends DisposableObject implements VariantA return this.views.get(variantAnalysisId); } + public async loadResults(variantAnalysisId: number, repositoryFullName: string): Promise { + const variantAnalysis = this.variantAnalyses.get(variantAnalysisId); + if (!variantAnalysis) { + throw new Error(`No variant analysis with id: ${variantAnalysisId}`); + } + + await this.variantAnalysisResultsManager.loadResults(variantAnalysisId, repositoryFullName); + } + private async onVariantAnalysisUpdated(variantAnalysis: VariantAnalysis | undefined): Promise { if (!variantAnalysis) { return; } + this.variantAnalyses.set(variantAnalysis.id, variantAnalysis); + await this.getView(variantAnalysis.id)?.updateView(variantAnalysis); } @@ -80,6 +94,10 @@ export class VariantAnalysisManager extends DisposableObject implements VariantA this._onVariantAnalysisAdded.fire(variantAnalysis); } + private async onRepoResultLoaded(repositoryResult: VariantAnalysisScannedRepositoryResult): Promise { + await this.getView(repositoryResult.variantAnalysisId)?.sendRepositoryResults([repositoryResult]); + } + private async onRepoStateUpdated(variantAnalysisId: number, repoState: VariantAnalysisScannedRepositoryState): Promise { await this.getView(variantAnalysisId)?.updateRepoState(repoState); } diff --git a/extensions/ql-vscode/src/remote-queries/variant-analysis-monitor.ts b/extensions/ql-vscode/src/remote-queries/variant-analysis-monitor.ts index 287f945e4..89a8231ff 100644 --- a/extensions/ql-vscode/src/remote-queries/variant-analysis-monitor.ts +++ b/extensions/ql-vscode/src/remote-queries/variant-analysis-monitor.ts @@ -41,6 +41,8 @@ export class VariantAnalysisMonitor extends DisposableObject { let attemptCount = 0; const scannedReposDownloaded: number[] = []; + this._onVariantAnalysisChange.fire(variantAnalysis); + while (attemptCount <= VariantAnalysisMonitor.maxAttemptCount) { await this.sleep(VariantAnalysisMonitor.sleepTime); 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 b4e2374fe..e9ec3e6ea 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 @@ -18,7 +18,7 @@ import { unzipFile } from '../pure/zip'; type CacheKey = `${number}/${string}`; -const createCacheKey = (variantAnalysisId: number, repoTask: VariantAnalysisRepoTask): CacheKey => `${variantAnalysisId}/${repoTask.repository.full_name}`; +const createCacheKey = (variantAnalysisId: number, repositoryFullName: string): CacheKey => `${variantAnalysisId}/${repositoryFullName}`; export type ResultDownloadedEvent = { variantAnalysisId: number; @@ -26,6 +26,9 @@ export type ResultDownloadedEvent = { } export class VariantAnalysisResultsManager extends DisposableObject { + private static readonly REPO_TASK_FILENAME = 'repo_task.json'; + private static readonly RESULTS_DIRECTORY = 'results'; + private readonly cachedResults: Map; private readonly _onResultDownloaded = this.push(new EventEmitter()); @@ -63,8 +66,10 @@ export class VariantAnalysisResultsManager extends DisposableObject { await fs.mkdir(resultDirectory, { recursive: true }); } + await fs.outputJson(path.join(resultDirectory, VariantAnalysisResultsManager.REPO_TASK_FILENAME), repoTask); + const zipFilePath = path.join(resultDirectory, 'results.zip'); - const unzippedFilesDirectory = path.join(resultDirectory, 'results'); + const unzippedFilesDirectory = path.join(resultDirectory, VariantAnalysisResultsManager.RESULTS_DIRECTORY); fs.writeFileSync(zipFilePath, Buffer.from(result)); await unzipFile(zipFilePath, unzippedFilesDirectory); @@ -77,41 +82,44 @@ export class VariantAnalysisResultsManager extends DisposableObject { public async loadResults( variantAnalysisId: number, - repoTask: VariantAnalysisRepoTask + repositoryFullName: string ): Promise { - const result = this.cachedResults.get(createCacheKey(variantAnalysisId, repoTask)); + const result = this.cachedResults.get(createCacheKey(variantAnalysisId, repositoryFullName)); - return result ?? await this.loadResultsIntoMemory(variantAnalysisId, repoTask); + return result ?? await this.loadResultsIntoMemory(variantAnalysisId, repositoryFullName); } private async loadResultsIntoMemory( variantAnalysisId: number, - repoTask: VariantAnalysisRepoTask, + repositoryFullName: string, ): Promise { - const result = await this.loadResultsFromStorage(variantAnalysisId, repoTask); - this.cachedResults.set(createCacheKey(variantAnalysisId, repoTask), result); + const result = await this.loadResultsFromStorage(variantAnalysisId, repositoryFullName); + this.cachedResults.set(createCacheKey(variantAnalysisId, repositoryFullName), result); this._onResultLoaded.fire(result); return result; } private async loadResultsFromStorage( variantAnalysisId: number, - repoTask: VariantAnalysisRepoTask, + repositoryFullName: string, ): Promise { - if (!(await this.isVariantAnalysisRepoDownloaded(variantAnalysisId, repoTask))) { + if (!(await this.isVariantAnalysisRepoDownloaded(variantAnalysisId, repositoryFullName))) { throw new Error('Variant analysis results not downloaded'); } + const storageDirectory = this.getRepoStorageDirectory(variantAnalysisId, repositoryFullName); + + const repoTask: VariantAnalysisRepoTask = await fs.readJson(path.join(storageDirectory, VariantAnalysisResultsManager.REPO_TASK_FILENAME)); + if (!repoTask.database_commit_sha || !repoTask.source_location_prefix) { throw new Error('Missing database commit SHA'); } const fileLinkPrefix = this.createGitHubDotcomFileLinkPrefix(repoTask.repository.full_name, repoTask.database_commit_sha); - const storageDirectory = this.getRepoStorageDirectory(variantAnalysisId, repoTask.repository.full_name); - - const sarifPath = path.join(storageDirectory, 'results.sarif'); - const bqrsPath = path.join(storageDirectory, 'results.bqrs'); + const resultsDirectory = path.join(storageDirectory, VariantAnalysisResultsManager.RESULTS_DIRECTORY); + const sarifPath = path.join(resultsDirectory, 'results.sarif'); + const bqrsPath = path.join(resultsDirectory, 'results.bqrs'); if (await fs.pathExists(sarifPath)) { const interpretedResults = await this.readSarifResults(sarifPath, fileLinkPrefix); @@ -137,9 +145,9 @@ export class VariantAnalysisResultsManager extends DisposableObject { private async isVariantAnalysisRepoDownloaded( variantAnalysisId: number, - repoTask: VariantAnalysisRepoTask, + repositoryFullName: string, ): Promise { - return await fs.pathExists(this.getRepoStorageDirectory(variantAnalysisId, repoTask.repository.full_name)); + return await fs.pathExists(this.getRepoStorageDirectory(variantAnalysisId, repositoryFullName)); } private async readBqrsResults(filePath: string, fileLinkPrefix: string, sourceLocationPrefix: string): Promise { diff --git a/extensions/ql-vscode/src/remote-queries/variant-analysis-view.ts b/extensions/ql-vscode/src/remote-queries/variant-analysis-view.ts index 03c6e78be..91c691b5a 100644 --- a/extensions/ql-vscode/src/remote-queries/variant-analysis-view.ts +++ b/extensions/ql-vscode/src/remote-queries/variant-analysis-view.ts @@ -1,4 +1,4 @@ -import { ExtensionContext, ViewColumn } from 'vscode'; +import { commands, ExtensionContext, ViewColumn } from 'vscode'; import { AbstractWebview, WebviewPanelConfig } from '../abstract-webview'; import { logger } from '../logging'; import { FromVariantAnalysisMessage, ToVariantAnalysisMessage } from '../pure/interface-types'; @@ -7,6 +7,7 @@ import { VariantAnalysis, VariantAnalysisQueryLanguage, VariantAnalysisRepoStatus, + VariantAnalysisScannedRepositoryResult, VariantAnalysisScannedRepositoryState, VariantAnalysisStatus } from './shared/variant-analysis'; @@ -53,6 +54,17 @@ export class VariantAnalysisView extends AbstractWebview { + if (!this.isShowingPanel) { + return; + } + + await this.postMessage({ + t: 'setRepoResults', + repoResults: repositoryResult, + }); + } + protected getPanelConfig(): WebviewPanelConfig { return { viewId: VariantAnalysisView.viewType, @@ -83,6 +95,9 @@ export class VariantAnalysisView extends AbstractWebview { const [isExpanded, setExpanded] = useState(false); + const resultsLoaded = !!interpretedResults || !!rawResults; + const [resultsLoading, setResultsLoading] = useState(false); - const toggleExpanded = useCallback(() => { - setExpanded(oldIsExpanded => !oldIsExpanded); - }, []); + const toggleExpanded = useCallback(async () => { + if (resultsLoading) { + return; + } + + if (resultsLoaded || status !== VariantAnalysisRepoStatus.Succeeded) { + setExpanded(oldIsExpanded => !oldIsExpanded); + return; + } + + vscode.postMessage({ + t: 'requestRepositoryResults', + repositoryFullName: repository.fullName, + }); + + setResultsLoading(true); + }, [resultsLoading, resultsLoaded, repository.fullName, status]); + + useEffect(() => { + if (resultsLoaded && resultsLoading) { + setResultsLoading(false); + setExpanded(true); + } + }, [resultsLoaded, resultsLoading]); const disabled = !status || !isCompletedAnalysisRepoStatus(status); + const expandableContentLoaded = status && (status !== VariantAnalysisRepoStatus.Succeeded || resultsLoaded); return (
@@ -107,7 +132,7 @@ export const RepoRow = ({ {downloadStatus === VariantAnalysisScannedRepositoryDownloadStatus.InProgress && } - {isExpanded && status && + {isExpanded && expandableContentLoaded && }
); diff --git a/extensions/ql-vscode/src/view/variant-analysis/__tests__/RepoRow.spec.tsx b/extensions/ql-vscode/src/view/variant-analysis/__tests__/RepoRow.spec.tsx index 3139a4db0..b7bd90f16 100644 --- a/extensions/ql-vscode/src/view/variant-analysis/__tests__/RepoRow.spec.tsx +++ b/extensions/ql-vscode/src/view/variant-analysis/__tests__/RepoRow.spec.tsx @@ -164,6 +164,56 @@ describe(RepoRow.name, () => { screen.getByText('Error: Timed out'); }); + it('can expand the repo item when succeeded and loaded', async () => { + render({ + status: VariantAnalysisRepoStatus.Succeeded, + interpretedResults: [], + }); + + await userEvent.click(screen.getByRole('button', { + expanded: false + })); + + expect(screen.getByRole('button', { + expanded: true, + })).toBeInTheDocument(); + }); + + it('can expand the repo item when succeeded and not loaded', async () => { + const { rerender } = render({ + status: VariantAnalysisRepoStatus.Succeeded, + }); + + await userEvent.click(screen.getByRole('button', { + expanded: false + })); + + expect((window as any).vsCodeApi.postMessage).toHaveBeenCalledWith({ + t: 'requestRepositoryResults', + repositoryFullName: 'octodemo/hello-world-1', + }); + + expect(screen.getByRole('button', { + expanded: false, + })).toBeInTheDocument(); + + rerender( + + ); + + expect(screen.getByRole('button', { + expanded: true, + })).toBeInTheDocument(); + }); + it('does not allow expanding the repo item when status is undefined', async () => { render({ status: undefined, diff --git a/extensions/ql-vscode/test/jest.setup.ts b/extensions/ql-vscode/test/jest.setup.ts index 2375aea05..a32b1b6a8 100644 --- a/extensions/ql-vscode/test/jest.setup.ts +++ b/extensions/ql-vscode/test/jest.setup.ts @@ -14,3 +14,11 @@ Object.defineProperty(window, 'matchMedia', { dispatchEvent: jest.fn(), })), }); + +// Store this on the window so we can mock it +(window as any).vsCodeApi = { + postMessage: jest.fn(), + setState: jest.fn(), +}; + +(window as any).acquireVsCodeApi = () => (window as any).vsCodeApi;