Merge pull request #1576 from github/elenatanasoiu/unzip

Make download method handle zip files

Author: elenatanasoiu

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

@@ -77,12 +77,9 @@ export async function getVariantAnalysisRepo(
 export async function getVariantAnalysisRepoResult(
   credentials: Credentials,
   downloadUrl: string,
-): Promise<unknown> {
+): Promise<ArrayBuffer> {
   const octokit = await credentials.getOctokit();
-
-  const response: OctokitResponse<VariantAnalysisRepoTask> = await octokit.request(
-    `GET ${downloadUrl}`
-  );
+  const response = await octokit.request(`GET ${downloadUrl}`);
 
   return response.data;
 }

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

@@ -14,6 +14,7 @@ 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';
 
 type CacheKey = `${number}/${string}`;
 
@@ -58,8 +59,15 @@ export class VariantAnalysisResultsManager extends DisposableObject {
       repoTask.artifact_url
     );
 
-    fs.mkdirSync(resultDirectory, { recursive: true });
-    await fs.writeFile(path.join(resultDirectory, 'results.zip'), JSON.stringify(result, null, 2), 'utf8');
+    if (!(await fs.pathExists(resultDirectory))) {
+      await fs.mkdir(resultDirectory, { recursive: true });
+    }
+
+    const zipFilePath = path.join(resultDirectory, 'results.zip');
+    const unzippedFilesDirectory = path.join(resultDirectory, 'results');
+
+    fs.writeFileSync(zipFilePath, Buffer.from(result));
+    await unzipFile(zipFilePath, unzippedFilesDirectory);
 
     this._onResultDownloaded.fire({
       variantAnalysisId,
@@ -156,7 +164,7 @@ export class VariantAnalysisResultsManager extends DisposableObject {
     );
   }
 
-  private getRepoStorageDirectory(variantAnalysisId: number, fullName: string): string {
+  public getRepoStorageDirectory(variantAnalysisId: number, fullName: string): string {
     return path.join(
       this.getStorageDirectory(variantAnalysisId),
       fullName

extensions/ql-vscode/src/vscode-tests/cli-integration/data/variant-analysis-results.zip

@@ -7,10 +7,12 @@ import * as config from '../../../config';
 import * as ghApiClient from '../../../remote-queries/gh-api/gh-api-client';
 import { Credentials } from '../../../authentication';
 import * as fs from 'fs-extra';
+import * as path from 'path';
 
 import { VariantAnalysisManager } from '../../../remote-queries/variant-analysis-manager';
 import {
   VariantAnalysis as VariantAnalysisApiResponse,
+  VariantAnalysisRepoTask,
   VariantAnalysisScannedRepository as ApiVariantAnalysisScannedRepository
 } from '../../../remote-queries/gh-api/variant-analysis';
 import { createMockApiResponse } from '../../factories/remote-queries/gh-api/variant-analysis-api-response';
@@ -72,6 +74,7 @@ describe('Variant Analysis Manager', async function() {
 
   describe('when credentials are valid', async () => {
     let getOctokitStub: sinon.SinonStub;
+    let arrayBuffer: ArrayBuffer;
 
     beforeEach(async () => {
       const mockCredentials = {
@@ -80,16 +83,18 @@ describe('Variant Analysis Manager', async function() {
         })
       } as unknown as Credentials;
       sandbox.stub(Credentials, 'initialize').resolves(mockCredentials);
+
+      const sourceFilePath = path.join(__dirname, '../../../../src/vscode-tests/cli-integration/data/variant-analysis-results.zip');
+      arrayBuffer = fs.readFileSync(sourceFilePath).buffer;
     });
 
     describe('when the artifact_url is missing', async () => {
       beforeEach(async () => {
         const dummyRepoTask = createMockVariantAnalysisRepoTask();
         delete dummyRepoTask.artifact_url;
-        getVariantAnalysisRepoStub = sandbox.stub(ghApiClient, 'getVariantAnalysisRepo').resolves(dummyRepoTask);
 
-        const dummyResult = 'this-is-a-repo-result';
-        getVariantAnalysisRepoResultStub = sandbox.stub(ghApiClient, 'getVariantAnalysisRepoResult').resolves(dummyResult);
+        getVariantAnalysisRepoStub = sandbox.stub(ghApiClient, 'getVariantAnalysisRepo').resolves(dummyRepoTask);
+        getVariantAnalysisRepoResultStub = sandbox.stub(ghApiClient, 'getVariantAnalysisRepoResult').resolves(arrayBuffer);
       });
 
       it('should not try to download the result', async () => {
@@ -104,12 +109,13 @@ describe('Variant Analysis Manager', async function() {
     });
 
     describe('when the artifact_url is present', async () => {
+      let dummyRepoTask: VariantAnalysisRepoTask;
+
       beforeEach(async () => {
-        const dummyRepoTask = createMockVariantAnalysisRepoTask();
-        getVariantAnalysisRepoStub = sandbox.stub(ghApiClient, 'getVariantAnalysisRepo').resolves(dummyRepoTask);
+        dummyRepoTask = createMockVariantAnalysisRepoTask();
 
-        const dummyResult = 'this-is-a-repo-result';
-        getVariantAnalysisRepoResultStub = sandbox.stub(ghApiClient, 'getVariantAnalysisRepoResult').resolves(dummyResult);
+        getVariantAnalysisRepoStub = sandbox.stub(ghApiClient, 'getVariantAnalysisRepo').resolves(dummyRepoTask);
+        getVariantAnalysisRepoResultStub = sandbox.stub(ghApiClient, 'getVariantAnalysisRepoResult').resolves(arrayBuffer);
       });
 
       it('should return early if variant analysis is cancelled', async () => {
@@ -143,16 +149,6 @@ describe('Variant Analysis Manager', async function() {
 
         expect(getVariantAnalysisRepoResultStub.calledOnce).to.be.true;
       });
-
-      it('should save the result to disk', async () => {
-        await variantAnalysisManager.autoDownloadVariantAnalysisResult(
-          scannedRepos[0],
-          variantAnalysis,
-          cancellationTokenSource.token
-        );
-
-        expect(getVariantAnalysisRepoResultStub.calledOnce).to.be.true;
-      });
     });
   });
 });

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

@@ -5,13 +5,15 @@ import { CodeQLExtensionInterface } from '../../../extension';
 import { logger } from '../../../logging';
 import { Credentials } from '../../../authentication';
 import * as fs from 'fs-extra';
+import * as path from 'path';
 
 import { VariantAnalysisResultsManager } from '../../../remote-queries/variant-analysis-results-manager';
 import { createMockVariantAnalysisRepoTask } from '../../factories/remote-queries/gh-api/variant-analysis-repo-task';
 import { CodeQLCliServer } from '../../../cli';
 import { storagePath } from '../global.helper';
 import { faker } from '@faker-js/faker';
 import * as ghApiClient from '../../../remote-queries/gh-api/gh-api-client';
+import { VariantAnalysisRepoTask } from '../../../remote-queries/gh-api/variant-analysis';
 
 describe(VariantAnalysisResultsManager.name, () => {
   let sandbox: sinon.SinonSandbox;
@@ -69,12 +71,29 @@ describe(VariantAnalysisResultsManager.name, () => {
     });
 
     describe('when the artifact_url is present', async () => {
-      it('should save the result to disk', async () => {
-        const dummyRepoTask = createMockVariantAnalysisRepoTask();
+      let dummyRepoTask: VariantAnalysisRepoTask;
+      let storageDirectory: string;
+      let arrayBuffer: ArrayBuffer;
+
+      beforeEach(async () => {
+        dummyRepoTask = createMockVariantAnalysisRepoTask();
+
+        storageDirectory = variantAnalysisResultsManager.getRepoStorageDirectory(variantAnalysisId, dummyRepoTask.repository.full_name);
+        const sourceFilePath = path.join(__dirname, '../../../../src/vscode-tests/cli-integration/data/variant-analysis-results.zip');
+        arrayBuffer = fs.readFileSync(sourceFilePath).buffer;
 
-        const dummyResult = 'this-is-a-repo-result';
-        getVariantAnalysisRepoResultStub = sandbox.stub(ghApiClient, 'getVariantAnalysisRepoResult').withArgs(mockCredentials, dummyRepoTask.artifact_url as string).resolves(dummyResult);
+        getVariantAnalysisRepoResultStub = sandbox
+          .stub(ghApiClient, 'getVariantAnalysisRepoResult')
+          .withArgs(mockCredentials, dummyRepoTask.artifact_url as string)
+          .resolves(arrayBuffer);
+      });
+
+      afterEach(async () => {
+        fs.removeSync(`${storageDirectory}/results.zip`);
+        fs.removeSync(`${storageDirectory}/results`);
+      });
 
+      it('should call the API to download the results', async () => {
         await variantAnalysisResultsManager.download(
           mockCredentials,
           variantAnalysisId,
@@ -83,6 +102,26 @@ describe(VariantAnalysisResultsManager.name, () => {
 
         expect(getVariantAnalysisRepoResultStub.calledOnce).to.be.true;
       });
+
+      it('should save the results zip file to disk', async () => {
+        await variantAnalysisResultsManager.download(
+          mockCredentials,
+          variantAnalysisId,
+          dummyRepoTask
+        );
+
+        expect(fs.existsSync(`${storageDirectory}/results.zip`)).to.be.true;
+      });
+
+      it('should unzip the results in a `results/` folder', async () => {
+        await variantAnalysisResultsManager.download(
+          mockCredentials,
+          variantAnalysisId,
+          dummyRepoTask
+        );
+
+        expect(fs.existsSync(`${storageDirectory}/results/results.sarif`)).to.be.true;
+      });
     });
   });
 });