Merge pull request #1716 from github/koesie10/use-shared-types-in-variant-analysis

Remove `gh-api` usage from variant analysis code

Author: koesie10

extensions/ql-vscode/src/extension.ts

@@ -109,11 +109,7 @@ import { NewQueryRunner } from './query-server/query-runner';
 import { QueryRunner } from './queryRunner';
 import { VariantAnalysisView } from './remote-queries/variant-analysis-view';
 import { VariantAnalysisViewSerializer } from './remote-queries/variant-analysis-view-serializer';
-import { VariantAnalysis } from './remote-queries/shared/variant-analysis';
-import {
-  VariantAnalysis as VariantAnalysisApiResponse,
-  VariantAnalysisScannedRepository as ApiVariantAnalysisScannedRepository
-} from './remote-queries/gh-api/variant-analysis';
+import { VariantAnalysis, VariantAnalysisScannedRepository } from './remote-queries/shared/variant-analysis';
 import { VariantAnalysisManager } from './remote-queries/variant-analysis-manager';
 import { createVariantAnalysisContentProvider } from './remote-queries/variant-analysis-content-provider';
 import { VSCodeMockGitHubApiServer } from './mocks/vscode-mock-gh-api-server';
@@ -949,8 +945,8 @@ async function activateWithInstalledDistribution(
 
   ctx.subscriptions.push(
     commandRunner('codeQL.autoDownloadVariantAnalysisResult', async (
-      scannedRepo: ApiVariantAnalysisScannedRepository,
-      variantAnalysisSummary: VariantAnalysisApiResponse,
+      scannedRepo: VariantAnalysisScannedRepository,
+      variantAnalysisSummary: VariantAnalysis,
       token: CancellationToken
     ) => {
       await variantAnalysisManager.enqueueDownload(scannedRepo, variantAnalysisSummary, token);

extensions/ql-vscode/src/remote-queries/shared/variant-analysis.ts

@@ -76,6 +76,17 @@ export interface VariantAnalysisScannedRepository {
   failureMessage?: string
 }
 
+export interface VariantAnalysisRepositoryTask {
+  repository: Repository,
+  analysisStatus: VariantAnalysisRepoStatus,
+  resultCount?: number,
+  artifactSizeInBytes?: number,
+  failureMessage?: string,
+  databaseCommitSha?: string,
+  sourceLocationPrefix?: string,
+  artifactUrl?: string,
+}
+
 export interface VariantAnalysisSkippedRepositories {
   accessMismatchRepos?: VariantAnalysisSkippedRepositoryGroup,
   notFoundRepos?: VariantAnalysisSkippedRepositoryGroup,

extensions/ql-vscode/src/remote-queries/variant-analysis-manager.ts

@@ -5,14 +5,11 @@ import { CancellationToken, commands, EventEmitter, ExtensionContext, window } f
 import { DisposableObject } from '../pure/disposable-object';
 import { Credentials } from '../authentication';
 import { VariantAnalysisMonitor } from './variant-analysis-monitor';
-import {
-  VariantAnalysis as VariantAnalysisApiResponse,
-  VariantAnalysisRepoTask,
-  VariantAnalysisScannedRepository as ApiVariantAnalysisScannedRepository
-} from './gh-api/variant-analysis';
 import {
   isVariantAnalysisComplete,
-  VariantAnalysis, VariantAnalysisQueryLanguage,
+  VariantAnalysis,
+  VariantAnalysisQueryLanguage,
+  VariantAnalysisRepositoryTask,
   VariantAnalysisScannedRepository,
   VariantAnalysisScannedRepositoryDownloadStatus,
   VariantAnalysisScannedRepositoryResult,
@@ -23,7 +20,7 @@ import { VariantAnalysisView } from './variant-analysis-view';
 import { VariantAnalysisViewManager } from './variant-analysis-view-manager';
 import { VariantAnalysisResultsManager } from './variant-analysis-results-manager';
 import { getControllerRepo } from './run-remote-query';
-import { processUpdatedVariantAnalysis } from './variant-analysis-processor';
+import { processUpdatedVariantAnalysis, processVariantAnalysisRepositoryTask } from './variant-analysis-processor';
 import PQueue from 'p-queue';
 import { createTimestampFile, showAndLogErrorMessage } from '../helpers';
 import * as fs from 'fs-extra';
@@ -179,57 +176,59 @@ export class VariantAnalysisManager extends DisposableObject implements VariantA
   }
 
   public async autoDownloadVariantAnalysisResult(
-    scannedRepo: ApiVariantAnalysisScannedRepository,
-    variantAnalysisSummary: VariantAnalysisApiResponse,
+    scannedRepo: VariantAnalysisScannedRepository,
+    variantAnalysis: VariantAnalysis,
     cancellationToken: CancellationToken
   ): Promise<void> {
     const repoState = {
       repositoryId: scannedRepo.repository.id,
       downloadStatus: VariantAnalysisScannedRepositoryDownloadStatus.Pending,
     };
 
-    await this.onRepoStateUpdated(variantAnalysisSummary.id, repoState);
+    await this.onRepoStateUpdated(variantAnalysis.id, repoState);
 
     const credentials = await Credentials.initialize(this.ctx);
     if (!credentials) { throw Error('Error authenticating with GitHub'); }
 
     if (cancellationToken && cancellationToken.isCancellationRequested) {
       repoState.downloadStatus = VariantAnalysisScannedRepositoryDownloadStatus.Failed;
-      await this.onRepoStateUpdated(variantAnalysisSummary.id, repoState);
+      await this.onRepoStateUpdated(variantAnalysis.id, repoState);
       return;
     }
 
-    let repoTask: VariantAnalysisRepoTask;
+    let repoTask: VariantAnalysisRepositoryTask;
     try {
-      repoTask = await ghApiClient.getVariantAnalysisRepo(
+      const repoTaskResponse = await ghApiClient.getVariantAnalysisRepo(
         credentials,
-        variantAnalysisSummary.controller_repo.id,
-        variantAnalysisSummary.id,
+        variantAnalysis.controllerRepo.id,
+        variantAnalysis.id,
         scannedRepo.repository.id
       );
+
+      repoTask = processVariantAnalysisRepositoryTask(repoTaskResponse);
     } catch (e) {
       repoState.downloadStatus = VariantAnalysisScannedRepositoryDownloadStatus.Failed;
-      await this.onRepoStateUpdated(variantAnalysisSummary.id, repoState);
-      throw new Error(`Could not download the results for variant analysis with id: ${variantAnalysisSummary.id}. Error: ${getErrorMessage(e)}`);
+      await this.onRepoStateUpdated(variantAnalysis.id, repoState);
+      throw new Error(`Could not download the results for variant analysis with id: ${variantAnalysis.id}. Error: ${getErrorMessage(e)}`);
     }
 
-    if (repoTask.artifact_url) {
+    if (repoTask.artifactUrl) {
       repoState.downloadStatus = VariantAnalysisScannedRepositoryDownloadStatus.InProgress;
-      await this.onRepoStateUpdated(variantAnalysisSummary.id, repoState);
+      await this.onRepoStateUpdated(variantAnalysis.id, repoState);
 
-      await this.variantAnalysisResultsManager.download(credentials, variantAnalysisSummary.id, repoTask, this.getVariantAnalysisStorageLocation(variantAnalysisSummary.id));
+      await this.variantAnalysisResultsManager.download(credentials, variantAnalysis.id, repoTask, this.getVariantAnalysisStorageLocation(variantAnalysis.id));
     }
 
     repoState.downloadStatus = VariantAnalysisScannedRepositoryDownloadStatus.Succeeded;
-    await this.onRepoStateUpdated(variantAnalysisSummary.id, repoState);
+    await this.onRepoStateUpdated(variantAnalysis.id, repoState);
   }
 
   public async enqueueDownload(
-    scannedRepo: ApiVariantAnalysisScannedRepository,
-    variantAnalysisSummary: VariantAnalysisApiResponse,
+    scannedRepo: VariantAnalysisScannedRepository,
+    variantAnalysis: VariantAnalysis,
     token: CancellationToken
   ): Promise<void> {
-    await this.queue.add(() => this.autoDownloadVariantAnalysisResult(scannedRepo, variantAnalysisSummary, token));
+    await this.queue.add(() => this.autoDownloadVariantAnalysisResult(scannedRepo, variantAnalysis, token));
   }
 
   public downloadsQueueSize(): number {

extensions/ql-vscode/src/remote-queries/variant-analysis-monitor.ts

@@ -1,12 +1,13 @@
-import { ExtensionContext, CancellationToken, commands, EventEmitter } from 'vscode';
+import { CancellationToken, commands, EventEmitter, ExtensionContext } from 'vscode';
 import { Credentials } from '../authentication';
 import * as ghApiClient from './gh-api/gh-api-client';
 
-import { isFinalVariantAnalysisStatus, VariantAnalysis } from './shared/variant-analysis';
 import {
-  VariantAnalysis as VariantAnalysisApiResponse,
+  isFinalVariantAnalysisStatus,
+  VariantAnalysis,
+  VariantAnalysisRepoStatus,
   VariantAnalysisScannedRepository
-} from './gh-api/variant-analysis';
+} from './shared/variant-analysis';
 import { VariantAnalysisMonitorResult } from './shared/variant-analysis-monitor-result';
 import { processUpdatedVariantAnalysis } from './variant-analysis-processor';
 import { DisposableObject } from '../pure/disposable-object';
@@ -57,7 +58,7 @@ export class VariantAnalysisMonitor extends DisposableObject {
 
       this._onVariantAnalysisChange.fire(variantAnalysis);
 
-      const downloadedRepos = this.downloadVariantAnalysisResults(variantAnalysisSummary, scannedReposDownloaded);
+      const downloadedRepos = this.downloadVariantAnalysisResults(variantAnalysis, scannedReposDownloaded);
       scannedReposDownloaded.push(...downloadedRepos);
 
       if (isFinalVariantAnalysisStatus(variantAnalysis.status) || variantAnalysis.failureReason) {
@@ -72,7 +73,7 @@ export class VariantAnalysisMonitor extends DisposableObject {
 
   private scheduleForDownload(
     scannedRepo: VariantAnalysisScannedRepository,
-    variantAnalysisSummary: VariantAnalysisApiResponse
+    variantAnalysisSummary: VariantAnalysis
   ) {
     void commands.executeCommand('codeQL.autoDownloadVariantAnalysisResult', scannedRepo, variantAnalysisSummary);
   }
@@ -81,22 +82,22 @@ export class VariantAnalysisMonitor extends DisposableObject {
     scannedRepo: VariantAnalysisScannedRepository,
     alreadyDownloaded: number[]
   ): boolean {
-    return !alreadyDownloaded.includes(scannedRepo.repository.id) && scannedRepo.analysis_status === 'succeeded';
+    return !alreadyDownloaded.includes(scannedRepo.repository.id) && scannedRepo.analysisStatus === VariantAnalysisRepoStatus.Succeeded;
   }
 
   private getReposToDownload(
-    variantAnalysisSummary: VariantAnalysisApiResponse,
+    variantAnalysisSummary: VariantAnalysis,
     alreadyDownloaded: number[]
   ): VariantAnalysisScannedRepository[] {
-    if (variantAnalysisSummary.scanned_repositories) {
-      return variantAnalysisSummary.scanned_repositories.filter(scannedRepo => this.shouldDownload(scannedRepo, alreadyDownloaded));
+    if (variantAnalysisSummary.scannedRepos) {
+      return variantAnalysisSummary.scannedRepos.filter(scannedRepo => this.shouldDownload(scannedRepo, alreadyDownloaded));
     } else {
       return [];
     }
   }
 
   private downloadVariantAnalysisResults(
-    variantAnalysisSummary: VariantAnalysisApiResponse,
+    variantAnalysisSummary: VariantAnalysis,
     scannedReposDownloaded: number[]
   ): number[] {
     const repoResultsToDownload = this.getReposToDownload(variantAnalysisSummary, scannedReposDownloaded);

extensions/ql-vscode/src/remote-queries/variant-analysis-processor.ts

@@ -6,7 +6,8 @@ import {
   VariantAnalysisFailureReason as ApiVariantAnalysisFailureReason,
   VariantAnalysisStatus as ApiVariantAnalysisStatus,
   VariantAnalysisSkippedRepositoryGroup as ApiVariantAnalysisSkippedRepositoryGroup,
-  VariantAnalysisNotFoundRepositoryGroup as ApiVariantAnalysisNotFoundRepositoryGroup
+  VariantAnalysisNotFoundRepositoryGroup as ApiVariantAnalysisNotFoundRepositoryGroup,
+  VariantAnalysisRepoTask as ApiVariantAnalysisRepoTask,
 } from './gh-api/variant-analysis';
 import {
   VariantAnalysis,
@@ -16,7 +17,8 @@ import {
   VariantAnalysisStatus,
   VariantAnalysisRepoStatus,
   VariantAnalysisSubmission,
-  VariantAnalysisSkippedRepositoryGroup
+  VariantAnalysisSkippedRepositoryGroup,
+  VariantAnalysisRepositoryTask
 } from './shared/variant-analysis';
 
 export function processVariantAnalysis(
@@ -76,24 +78,47 @@ export function processUpdatedVariantAnalysis(
   return variantAnalysis;
 }
 
+export function processVariantAnalysisRepositoryTask(
+  response: ApiVariantAnalysisRepoTask
+): VariantAnalysisRepositoryTask {
+  return {
+    repository: {
+      id: response.repository.id,
+      fullName: response.repository.full_name,
+      private: response.repository.private,
+    },
+    analysisStatus: processApiRepoStatus(response.analysis_status),
+    resultCount: response.result_count,
+    artifactSizeInBytes: response.artifact_size_in_bytes,
+    failureMessage: response.failure_message,
+    databaseCommitSha: response.database_commit_sha,
+    sourceLocationPrefix: response.source_location_prefix,
+    artifactUrl: response.artifact_url,
+  };
+}
+
+export function processScannedRepository(
+  scannedRepo: ApiVariantAnalysisScannedRepository
+): VariantAnalysisScannedRepository {
+  return {
+    repository: {
+      id: scannedRepo.repository.id,
+      fullName: scannedRepo.repository.full_name,
+      private: scannedRepo.repository.private,
+      stargazersCount: scannedRepo.repository.stargazers_count,
+      updatedAt: scannedRepo.repository.updated_at,
+    },
+    analysisStatus: processApiRepoStatus(scannedRepo.analysis_status),
+    resultCount: scannedRepo.result_count,
+    artifactSizeInBytes: scannedRepo.artifact_size_in_bytes,
+    failureMessage: scannedRepo.failure_message
+  };
+}
+
 function processScannedRepositories(
   scannedRepos: ApiVariantAnalysisScannedRepository[]
 ): VariantAnalysisScannedRepository[] {
-  return scannedRepos.map(scannedRepo => {
-    return {
-      repository: {
-        id: scannedRepo.repository.id,
-        fullName: scannedRepo.repository.full_name,
-        private: scannedRepo.repository.private,
-        stargazersCount: scannedRepo.repository.stargazers_count,
-        updatedAt: scannedRepo.repository.updated_at,
-      },
-      analysisStatus: processApiRepoStatus(scannedRepo.analysis_status),
-      resultCount: scannedRepo.result_count,
-      artifactSizeInBytes: scannedRepo.artifact_size_in_bytes,
-      failureMessage: scannedRepo.failure_message
-    };
-  });
+  return scannedRepos.map(scannedRepo => processScannedRepository(scannedRepo));
 }
 
 function processSkippedRepositories(

extensions/ql-vscode/src/remote-queries/variant-analysis-results-manager.ts

@@ -9,9 +9,12 @@ import { sarifParser } from '../sarif-parser';
 import { extractAnalysisAlerts } from './sarif-processing';
 import { CodeQLCliServer } from '../cli';
 import { extractRawResults } from './bqrs-processing';
-import { VariantAnalysis, VariantAnalysisScannedRepositoryResult } from './shared/variant-analysis';
+import {
+  VariantAnalysis,
+  VariantAnalysisRepositoryTask,
+  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';
 import { EventEmitter } from 'vscode';
 import { unzipFile } from '../pure/zip';
@@ -22,7 +25,7 @@ const createCacheKey = (variantAnalysisId: number, repositoryFullName: string):
 
 export type ResultDownloadedEvent = {
   variantAnalysisId: number;
-  repoTask: VariantAnalysisRepoTask;
+  repoTask: VariantAnalysisRepositoryTask;
 }
 
 export class VariantAnalysisResultsManager extends DisposableObject {
@@ -48,18 +51,18 @@ export class VariantAnalysisResultsManager extends DisposableObject {
   public async download(
     credentials: Credentials,
     variantAnalysisId: number,
-    repoTask: VariantAnalysisRepoTask,
+    repoTask: VariantAnalysisRepositoryTask,
     variantAnalysisStoragePath: string,
   ): Promise<void> {
-    if (!repoTask.artifact_url) {
+    if (!repoTask.artifactUrl) {
       throw new Error('Missing artifact URL');
     }
 
-    const resultDirectory = this.getRepoStorageDirectory(variantAnalysisStoragePath, repoTask.repository.full_name);
+    const resultDirectory = this.getRepoStorageDirectory(variantAnalysisStoragePath, repoTask.repository.fullName);
 
     const result = await ghApiClient.getVariantAnalysisRepoResult(
       credentials,
-      repoTask.artifact_url
+      repoTask.artifactUrl
     );
 
     if (!(await fs.pathExists(resultDirectory))) {
@@ -112,13 +115,13 @@ export class VariantAnalysisResultsManager extends DisposableObject {
 
     const storageDirectory = this.getRepoStorageDirectory(variantAnalysisStoragePath, repositoryFullName);
 
-    const repoTask: VariantAnalysisRepoTask = await fs.readJson(path.join(storageDirectory, VariantAnalysisResultsManager.REPO_TASK_FILENAME));
+    const repoTask: VariantAnalysisRepositoryTask = await fs.readJson(path.join(storageDirectory, VariantAnalysisResultsManager.REPO_TASK_FILENAME));
 
-    if (!repoTask.database_commit_sha || !repoTask.source_location_prefix) {
+    if (!repoTask.databaseCommitSha || !repoTask.sourceLocationPrefix) {
       throw new Error('Missing database commit SHA');
     }
 
-    const fileLinkPrefix = this.createGitHubDotcomFileLinkPrefix(repoTask.repository.full_name, repoTask.database_commit_sha);
+    const fileLinkPrefix = this.createGitHubDotcomFileLinkPrefix(repoTask.repository.fullName, repoTask.databaseCommitSha);
 
     const resultsDirectory = path.join(storageDirectory, VariantAnalysisResultsManager.RESULTS_DIRECTORY);
     const sarifPath = path.join(resultsDirectory, 'results.sarif');
@@ -134,7 +137,7 @@ export class VariantAnalysisResultsManager extends DisposableObject {
     }
 
     if (await fs.pathExists(bqrsPath)) {
-      const rawResults = await this.readBqrsResults(bqrsPath, fileLinkPrefix, repoTask.source_location_prefix);
+      const rawResults = await this.readBqrsResults(bqrsPath, fileLinkPrefix, repoTask.sourceLocationPrefix);
 
       return {
         variantAnalysisId,