Output percentage during results download

Author: robertbrignull

extensions/ql-vscode/src/extension.ts

@@ -633,7 +633,6 @@ async function activateWithInstalledDistribution(
   );
   await ensureDir(variantAnalysisStorageDir);
   const variantAnalysisResultsManager = new VariantAnalysisResultsManager(
-    app.credentials,
     cliServer,
     extLogger,
   );

extensions/ql-vscode/src/remote-queries/gh-api/gh-api-client.ts

@@ -81,16 +81,6 @@ export async function getVariantAnalysisRepo(
   return response.data;
 }
 
-export async function getVariantAnalysisRepoResult(
-  credentials: Credentials,
-  downloadUrl: string,
-): Promise<ArrayBuffer> {
-  const octokit = await credentials.getOctokit();
-  const response = await octokit.request(`GET ${downloadUrl}`);
-
-  return response.data;
-}
-
 export async function getRepositoryFromNwo(
   credentials: Credentials,
   owner: string,

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

@@ -68,6 +68,7 @@ export class VariantAnalysisManager
   implements VariantAnalysisViewManager<VariantAnalysisView>
 {
   private static readonly REPO_STATES_FILENAME = "repo_states.json";
+  private static readonly DOWNLOAD_PERCENTAGE_UPDATE_DELAY_MS = 3000;
 
   private readonly _onVariantAnalysisAdded = this.push(
     new EventEmitter<VariantAnalysis>(),
@@ -514,10 +515,27 @@ export class VariantAnalysisManager
       await this.onRepoStateUpdated(variantAnalysis.id, repoState);
 
       try {
+        let lastRepoStateUpdate = 0;
+        const updateRepoStateCallback = async (downloadPercentage: number) => {
+          const now = new Date().getTime();
+          if (
+            lastRepoStateUpdate <
+            now - VariantAnalysisManager.DOWNLOAD_PERCENTAGE_UPDATE_DELAY_MS
+          ) {
+            lastRepoStateUpdate = now;
+            await this.onRepoStateUpdated(variantAnalysis.id, {
+              repositoryId: scannedRepo.repository.id,
+              downloadStatus:
+                VariantAnalysisScannedRepositoryDownloadStatus.InProgress,
+              downloadPercentage,
+            });
+          }
+        };
         await this.variantAnalysisResultsManager.download(
           variantAnalysis.id,
           repoTask,
           this.getVariantAnalysisStorageLocation(variantAnalysis.id),
+          updateRepoStateCallback,
         );
       } catch (e) {
         repoState.downloadStatus =

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

@@ -1,14 +1,14 @@
 import {
+  appendFileSync,
   pathExists,
   mkdir,
   outputJson,
-  writeFileSync,
   readJson,
 } from "fs-extra";
+import fetch from "node-fetch";
 import { EOL } from "os";
 import { join } from "path";
 
-import { Credentials } from "../common/authentication";
 import { Logger } from "../common";
 import { AnalysisAlert, AnalysisRawResults } from "./shared/analysis-result";
 import { sarifParser } from "../sarif-parser";
@@ -21,7 +21,6 @@ import {
   VariantAnalysisScannedRepositoryResult,
 } from "./shared/variant-analysis";
 import { DisposableObject, DisposeHandler } from "../pure/disposable-object";
-import { getVariantAnalysisRepoResult } from "./gh-api/gh-api-client";
 import { EventEmitter } from "vscode";
 import { unzipFile } from "../pure/zip";
 
@@ -63,7 +62,6 @@ export class VariantAnalysisResultsManager extends DisposableObject {
   readonly onResultLoaded = this._onResultLoaded.event;
 
   constructor(
-    private readonly credentials: Credentials,
     private readonly cliServer: CodeQLCliServer,
     private readonly logger: Logger,
   ) {
@@ -75,6 +73,7 @@ export class VariantAnalysisResultsManager extends DisposableObject {
     variantAnalysisId: number,
     repoTask: VariantAnalysisRepositoryTask,
     variantAnalysisStoragePath: string,
+    onDownloadPercentageChanged: (downloadPercentage: number) => Promise<void>,
   ): Promise<void> {
     if (!repoTask.artifactUrl) {
       throw new Error("Missing artifact URL");
@@ -85,11 +84,6 @@ export class VariantAnalysisResultsManager extends DisposableObject {
       repoTask.repository.fullName,
     );
 
-    const result = await getVariantAnalysisRepoResult(
-      this.credentials,
-      repoTask.artifactUrl,
-    );
-
     if (!(await pathExists(resultDirectory))) {
       await mkdir(resultDirectory, { recursive: true });
     }
@@ -100,12 +94,22 @@ export class VariantAnalysisResultsManager extends DisposableObject {
     );
 
     const zipFilePath = join(resultDirectory, "results.zip");
+
+    const response = await fetch(repoTask.artifactUrl);
+    let amountDownloaded = 0;
+    for await (const chunk of response.body) {
+      appendFileSync(zipFilePath, Buffer.from(chunk));
+      amountDownloaded += chunk.length;
+      await onDownloadPercentageChanged(
+        Math.floor((amountDownloaded / response.size) * 100),
+      );
+    }
+
     const unzippedFilesDirectory = join(
       resultDirectory,
       VariantAnalysisResultsManager.RESULTS_DIRECTORY,
     );
 
-    writeFileSync(zipFilePath, Buffer.from(result));
     await unzipFile(zipFilePath, unzippedFilesDirectory);
 
     this._onResultDownloaded.fire({

extensions/ql-vscode/test/unit-tests/remote-queries/gh-api/gh-api-client.test.ts

@@ -1,10 +1,7 @@
-import { faker } from "@faker-js/faker";
-
 import {
   getRepositoryFromNwo,
   getVariantAnalysis,
   getVariantAnalysisRepo,
-  getVariantAnalysisRepoResult,
   submitVariantAnalysis,
 } from "../../../../src/remote-queries/gh-api/gh-api-client";
 import { createMockSubmission } from "../../../factories/remote-queries/shared/variant-analysis-submission";
@@ -69,23 +66,6 @@ describe("getVariantAnalysisRepo", () => {
   });
 });
 
-describe("getVariantAnalysisRepoResult", () => {
-  it("returns the variant analysis repo result", async () => {
-    await mockServer.loadScenario("problem-query-success");
-
-    const result = await getVariantAnalysisRepoResult(
-      testCredentialsWithRealOctokit(),
-      `https://objects-origin.githubusercontent.com/codeql-query-console/codeql-variant-analysis-repo-tasks/${variantAnalysisId}/${repoTaskId}/${faker.datatype.uuid()}`,
-    );
-
-    expect(result).toBeDefined();
-    expect(result).toBeInstanceOf(ArrayBuffer);
-    expect(result.byteLength).toBe(
-      variantAnalysisRepoJson_response.body.artifact_size_in_bytes,
-    );
-  });
-});
-
 describe("getRepositoryFromNwo", () => {
   it("returns the repository", async () => {
     await mockServer.loadScenario("problem-query-success");

extensions/ql-vscode/test/vscode-tests/cli-integration/remote-queries/variant-analysis-manager.test.ts

@@ -21,6 +21,8 @@ import * as ghApiClient from "../../../../src/remote-queries/gh-api/gh-api-clien
 import * as ghActionsApiClient from "../../../../src/remote-queries/gh-api/gh-actions-api-client";
 import * as fs from "fs-extra";
 import { join } from "path";
+import { Response } from "node-fetch";
+import * as fetchModule from "node-fetch";
 
 import { VariantAnalysisManager } from "../../../../src/remote-queries/variant-analysis-manager";
 import { CodeQLCliServer } from "../../../../src/cli";
@@ -95,7 +97,6 @@ describe("Variant Analysis Manager", () => {
     cli = extension.cliServer;
     app = createMockApp({});
     variantAnalysisResultsManager = new VariantAnalysisResultsManager(
-      app.credentials,
       cli,
       extLogger,
     );
@@ -334,32 +335,21 @@ describe("Variant Analysis Manager", () => {
   });
 
   describe("autoDownloadVariantAnalysisResult", () => {
-    let arrayBuffer: ArrayBuffer;
-
     let getVariantAnalysisRepoStub: jest.SpiedFunction<
       typeof ghApiClient.getVariantAnalysisRepo
     >;
     let getVariantAnalysisRepoResultStub: jest.SpiedFunction<
-      typeof ghApiClient.getVariantAnalysisRepoResult
+      typeof fetchModule.default
     >;
 
     let repoStatesPath: string;
 
     beforeEach(async () => {
-      const sourceFilePath = join(
-        __dirname,
-        "../data/variant-analysis-results.zip",
-      );
-      arrayBuffer = fs.readFileSync(sourceFilePath).buffer;
-
       getVariantAnalysisRepoStub = jest.spyOn(
         ghApiClient,
         "getVariantAnalysisRepo",
       );
-      getVariantAnalysisRepoResultStub = jest.spyOn(
-        ghApiClient,
-        "getVariantAnalysisRepoResult",
-      );
+      getVariantAnalysisRepoResultStub = jest.spyOn(fetchModule, "default");
 
       repoStatesPath = join(
         storagePath,
@@ -374,7 +364,6 @@ describe("Variant Analysis Manager", () => {
         delete dummyRepoTask.artifact_url;
 
         getVariantAnalysisRepoStub.mockResolvedValue(dummyRepoTask);
-        getVariantAnalysisRepoResultStub.mockResolvedValue(arrayBuffer);
       });
 
       it("should not try to download the result", async () => {
@@ -395,7 +384,15 @@ describe("Variant Analysis Manager", () => {
         dummyRepoTask = createMockVariantAnalysisRepoTask();
 
         getVariantAnalysisRepoStub.mockResolvedValue(dummyRepoTask);
-        getVariantAnalysisRepoResultStub.mockResolvedValue(arrayBuffer);
+
+        const sourceFilePath = join(
+          __dirname,
+          "../data/variant-analysis-results.zip",
+        );
+        const arrayBuffer = fs.readFileSync(sourceFilePath).buffer;
+        getVariantAnalysisRepoResultStub.mockResolvedValue(
+          new Response(arrayBuffer),
+        );
       });
 
       it("should return early if variant analysis is cancelled", async () => {

extensions/ql-vscode/test/vscode-tests/cli-integration/remote-queries/variant-analysis-results-manager.test.ts

@@ -3,19 +3,18 @@ import { CodeQLExtensionInterface } from "../../../../src/extension";
 import { extLogger } from "../../../../src/common";
 import * as fs from "fs-extra";
 import { join, resolve } from "path";
+import { Response, RequestInfo, RequestInit } from "node-fetch";
+import * as fetchModule from "node-fetch";
 
 import { VariantAnalysisResultsManager } from "../../../../src/remote-queries/variant-analysis-results-manager";
 import { CodeQLCliServer } from "../../../../src/cli";
 import { storagePath } from "../global.helper";
 import { faker } from "@faker-js/faker";
-import * as ghApiClient from "../../../../src/remote-queries/gh-api/gh-api-client";
 import { createMockVariantAnalysisRepositoryTask } from "../../../factories/remote-queries/shared/variant-analysis-repo-tasks";
 import {
   VariantAnalysisRepositoryTask,
   VariantAnalysisScannedRepositoryResult,
 } from "../../../../src/remote-queries/shared/variant-analysis";
-import { testCredentialsWithStub } from "../../../factories/authentication";
-import { Credentials } from "../../../../src/common/authentication";
 
 jest.setTimeout(10_000);
 
@@ -44,7 +43,6 @@ describe(VariantAnalysisResultsManager.name, () => {
       jest.spyOn(extLogger, "log").mockResolvedValue(undefined);
 
       variantAnalysisResultsManager = new VariantAnalysisResultsManager(
-        testCredentialsWithStub(),
         cli,
         extLogger,
       );
@@ -89,6 +87,7 @@ describe(VariantAnalysisResultsManager.name, () => {
             variantAnalysisId,
             dummyRepoTask,
             variantAnalysisStoragePath,
+            () => Promise.resolve(),
           ),
         ).rejects.toThrow("Missing artifact URL");
       });
@@ -98,7 +97,7 @@ describe(VariantAnalysisResultsManager.name, () => {
       let arrayBuffer: ArrayBuffer;
 
       let getVariantAnalysisRepoResultStub: jest.SpiedFunction<
-        typeof ghApiClient.getVariantAnalysisRepoResult
+        typeof fetchModule.default
       >;
 
       beforeEach(async () => {
@@ -109,22 +108,21 @@ describe(VariantAnalysisResultsManager.name, () => {
         arrayBuffer = fs.readFileSync(sourceFilePath).buffer;
 
         getVariantAnalysisRepoResultStub = jest
-          .spyOn(ghApiClient, "getVariantAnalysisRepoResult")
-          .mockImplementation(
-            (_credentials: Credentials, downloadUrl: string) => {
-              if (downloadUrl === dummyRepoTask.artifactUrl) {
-                return Promise.resolve(arrayBuffer);
-              }
-              return Promise.reject(new Error("Unexpected artifact URL"));
-            },
-          );
+          .spyOn(fetchModule, "default")
+          .mockImplementation((url: RequestInfo, _init?: RequestInit) => {
+            if (url === dummyRepoTask.artifactUrl) {
+              return Promise.resolve(new Response(arrayBuffer));
+            }
+            return Promise.reject(new Error("Unexpected artifact URL"));
+          });
       });
 
       it("should call the API to download the results", async () => {
         await variantAnalysisResultsManager.download(
           variantAnalysisId,
           dummyRepoTask,
           variantAnalysisStoragePath,
+          () => Promise.resolve(),
         );
 
         expect(getVariantAnalysisRepoResultStub).toHaveBeenCalledTimes(1);
@@ -135,6 +133,7 @@ describe(VariantAnalysisResultsManager.name, () => {
           variantAnalysisId,
           dummyRepoTask,
           variantAnalysisStoragePath,
+          () => Promise.resolve(),
         );
 
         expect(fs.existsSync(`${repoTaskStorageDirectory}/results.zip`)).toBe(
@@ -147,6 +146,7 @@ describe(VariantAnalysisResultsManager.name, () => {
           variantAnalysisId,
           dummyRepoTask,
           variantAnalysisStoragePath,
+          () => Promise.resolve(),
         );
 
         expect(
@@ -160,6 +160,7 @@ describe(VariantAnalysisResultsManager.name, () => {
             variantAnalysisId,
             dummyRepoTask,
             variantAnalysisStoragePath,
+            () => Promise.resolve(),
           );
 
           expect(
@@ -185,7 +186,6 @@ describe(VariantAnalysisResultsManager.name, () => {
 
     beforeEach(() => {
       variantAnalysisResultsManager = new VariantAnalysisResultsManager(
-        testCredentialsWithStub(),
         cli,
         extLogger,
       );