Merge pull request #1572 from github/koesie10/request-repo-results-message

Implement `requestRepoResults` message

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) => {

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;

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<number, VariantAnalysis>();
   private readonly views = new Map<number, VariantAnalysisView>();
 
   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<void> {
@@ -68,11 +71,22 @@ 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}`);
+    }
+
+    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);
   }
 
@@ -80,6 +94,10 @@ export class VariantAnalysisManager extends DisposableObject implements VariantA
     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);
   }

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);
 

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<CacheKey, VariantAnalysisScannedRepositoryResult>;
 
   private readonly _onResultDownloaded = this.push(new EventEmitter<ResultDownloadedEvent>());
@@ -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<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);
+    const result = await this.loadResultsFromStorage(variantAnalysisId, repositoryFullName);
-    this.cachedResults.set(createCacheKey(variantAnalysisId, repoTask), result);
+    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 resultsDirectory = path.join(storageDirectory, VariantAnalysisResultsManager.RESULTS_DIRECTORY);
-
+    const sarifPath = path.join(resultsDirectory, 'results.sarif');
-    const sarifPath = path.join(storageDirectory, 'results.sarif');
+    const bqrsPath = path.join(resultsDirectory, 'results.bqrs');
-    const bqrsPath = path.join(storageDirectory, '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<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> {

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<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,
@@ -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);
     }

extensions/ql-vscode/src/view/variant-analysis/RepoRow.tsx

@@ -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 {
@@ -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
@@ -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(() => {
+  const toggleExpanded = useCallback(async () => {
-    setExpanded(oldIsExpanded => !oldIsExpanded);
+    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>
@@ -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>
   );

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(
+      <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,

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;