Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions extensions/ql-vscode/src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
8 changes: 7 additions & 1 deletion extensions/ql-vscode/src/pure/interface-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -459,11 +459,17 @@ export interface SetRepoStatesMessage {
repoStates: VariantAnalysisScannedRepositoryState[];
}

export interface RequestRepositoryResultsMessage {
t: 'requestRepositoryResults';
repositoryFullName: string;
}

export type ToVariantAnalysisMessage =
| SetVariantAnalysisMessage
| SetRepoResultsMessage
| SetRepoStatesMessage;

export type FromVariantAnalysisMessage =
| ViewLoadedMsg
| StopVariantAnalysisMessage;
| StopVariantAnalysisMessage
| RequestRepositoryResultsMessage;
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
import {
VariantAnalysis,
VariantAnalysisScannedRepositoryDownloadStatus,
VariantAnalysisScannedRepositoryResult,
VariantAnalysisScannedRepositoryState
} from './shared/variant-analysis';
import { getErrorMessage } from '../pure/helpers-pure';
Expand All @@ -26,6 +27,7 @@ export class VariantAnalysisManager extends DisposableObject implements VariantA

private readonly variantAnalysisMonitor: VariantAnalysisMonitor;
private readonly variantAnalysisResultsManager: VariantAnalysisResultsManager;
private readonly variantAnalyses = new Map<number, VariantAnalysis>();
private readonly views = new Map<number, VariantAnalysisView>();

constructor(
Expand All @@ -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<void> {
Expand Down Expand Up @@ -68,18 +71,33 @@ export class VariantAnalysisManager extends DisposableObject implements VariantA
return this.views.get(variantAnalysisId);
}

public async loadResults(variantAnalysisId: number, repositoryFullName: string): Promise<void> {
const variantAnalysis = this.variantAnalyses.get(variantAnalysisId);
if (!variantAnalysis) {
throw new Error(`No variant analysis with id: ${variantAnalysisId}`);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of interest, how would this error surface to the user? Would it appear as a notification or be in a debug window or be completely hidden?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is called by a command runner, this will show a warning notification and be logged (using the showAndLogWarningMessage function).

}

await this.variantAnalysisResultsManager.loadResults(variantAnalysisId, repositoryFullName);
}

private async onVariantAnalysisUpdated(variantAnalysis: VariantAnalysis | undefined): Promise<void> {
if (!variantAnalysis) {
return;
}

this.variantAnalyses.set(variantAnalysis.id, variantAnalysis);

await this.getView(variantAnalysis.id)?.updateView(variantAnalysis);
}

public onVariantAnalysisSubmitted(variantAnalysis: VariantAnalysis): void {
this._onVariantAnalysisAdded.fire(variantAnalysis);
}

private async onRepoResultLoaded(repositoryResult: VariantAnalysisScannedRepositoryResult): Promise<void> {
await this.getView(repositoryResult.variantAnalysisId)?.sendRepositoryResults([repositoryResult]);
}

private async onRepoStateUpdated(variantAnalysisId: number, repoState: VariantAnalysisScannedRepositoryState): Promise<void> {
await this.getView(variantAnalysisId)?.updateRepoState(repoState);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,17 @@ 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;
repoTask: VariantAnalysisRepoTask;
}

export class VariantAnalysisResultsManager extends DisposableObject {
private static readonly REPO_TASK_FILENAME = 'repo_task.json';
private static readonly RESULTS_DIRECTORY = 'results';

private readonly cachedResults: Map<CacheKey, VariantAnalysisScannedRepositoryResult>;

private readonly _onResultDownloaded = this.push(new EventEmitter<ResultDownloadedEvent>());
Expand Down Expand Up @@ -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);
Expand All @@ -77,41 +82,44 @@ export class VariantAnalysisResultsManager extends DisposableObject {

public async loadResults(
variantAnalysisId: number,
repoTask: VariantAnalysisRepoTask
repositoryFullName: string
): Promise<VariantAnalysisScannedRepositoryResult> {
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<VariantAnalysisScannedRepositoryResult> {
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<VariantAnalysisScannedRepositoryResult> {
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);

Expand All @@ -137,9 +145,9 @@ export class VariantAnalysisResultsManager extends DisposableObject {

private async isVariantAnalysisRepoDownloaded(
variantAnalysisId: number,
repoTask: VariantAnalysisRepoTask,
repositoryFullName: string,
): Promise<boolean> {
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<AnalysisRawResults> {
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -7,6 +7,7 @@ import {
VariantAnalysis,
VariantAnalysisQueryLanguage,
VariantAnalysisRepoStatus,
VariantAnalysisScannedRepositoryResult,
VariantAnalysisScannedRepositoryState,
VariantAnalysisStatus
} from './shared/variant-analysis';
Expand Down Expand Up @@ -53,6 +54,17 @@ export class VariantAnalysisView extends AbstractWebview<ToVariantAnalysisMessag
});
}

public async sendRepositoryResults(repositoryResult: VariantAnalysisScannedRepositoryResult[]): Promise<void> {
if (!this.isShowingPanel) {
return;
}

await this.postMessage({
t: 'setRepoResults',
repoResults: repositoryResult,
});
}

protected getPanelConfig(): WebviewPanelConfig {
return {
viewId: VariantAnalysisView.viewType,
Expand Down Expand Up @@ -83,6 +95,9 @@ export class VariantAnalysisView extends AbstractWebview<ToVariantAnalysisMessag
case 'stopVariantAnalysis':
void logger.log(`Stop variant analysis: ${msg.variantAnalysisId}`);
break;
case 'requestRepositoryResults':
void commands.executeCommand('codeQL.loadVariantAnalysisRepoResults', this.variantAnalysisId, msg.repositoryFullName);
break;
default:
assertNever(msg);
}
Expand Down
35 changes: 30 additions & 5 deletions extensions/ql-vscode/src/view/variant-analysis/RepoRow.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as React from 'react';
import { useCallback, useState } from 'react';
import { useCallback, useEffect, useState } from 'react';
import styled from 'styled-components';
import { VSCodeBadge, VSCodeCheckbox } from '@vscode/webview-ui-toolkit/react';
import {
Expand All @@ -11,6 +11,7 @@ import { formatDecimal } from '../../pure/number';
import { Codicon, ErrorIcon, LoadingIcon, SuccessIcon, WarningIcon } from '../common';
import { Repository } from '../../remote-queries/shared/repository';
import { AnalysisAlert, AnalysisRawResults } from '../../remote-queries/shared/analysis-result';
import { vscode } from '../vscode-api';
import { AnalyzedRepoItemContent } from './AnalyzedRepoItemContent';

// This will ensure that these icons have a className which we can use in the TitleContainer
Expand Down Expand Up @@ -82,12 +83,36 @@ export const RepoRow = ({
rawResults,
}: RepoRowProps) => {
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 (
<div>
Expand All @@ -107,7 +132,7 @@ export const RepoRow = ({
</span>
{downloadStatus === VariantAnalysisScannedRepositoryDownloadStatus.InProgress && <LoadingIcon label="Downloading" />}
</TitleContainer>
{isExpanded && status &&
{isExpanded && expandableContentLoaded &&
<AnalyzedRepoItemContent status={status} interpretedResults={interpretedResults} rawResults={rawResults} />}
</div>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
<RepoRow
repository={{
id: 1,
fullName: 'octodemo/hello-world-1',
private: false,
}}
status={VariantAnalysisRepoStatus.Succeeded}
interpretedResults={[]}
/>
);

expect(screen.getByRole('button', {
expanded: true,
})).toBeInTheDocument();
});

it('does not allow expanding the repo item when status is undefined', async () => {
render({
status: undefined,
Expand Down
8 changes: 8 additions & 0 deletions extensions/ql-vscode/test/jest.setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;