Merge pull request #2297 from github/elena/improvements-to-wizard

Refactor `digForDatabaseItem` and `digForDatabaseWithSameLanguage` methods for skeleton wizard

Author: elenatanasoiu

extensions/ql-vscode/src/local-databases.ts

@@ -897,31 +897,6 @@ export class DatabaseManager extends DisposableObject {
     }
   }
 
-  public async digForDatabaseItem(
-    language: string,
-    databaseNwo: string,
-  ): Promise<DatabaseItem | undefined> {
-    const dbItems = this.databaseItems || [];
-    const dbs = dbItems.filter(
-      (db) => db.language === language && db.name === databaseNwo,
-    );
-    if (dbs.length === 0) {
-      return undefined;
-    }
-    return dbs[0];
-  }
-
-  public async digForDatabaseWithSameLanguage(
-    language: string,
-  ): Promise<DatabaseItem | undefined> {
-    const dbItems = this.databaseItems || [];
-    const dbs = dbItems.filter((db) => db.language === language);
-    if (dbs.length === 0) {
-      return undefined;
-    }
-    return dbs[0];
-  }
-
   /**
    * Returns the index of the workspace folder that corresponds to the source archive of `item`
    * if there is one, and -1 otherwise.

extensions/ql-vscode/src/skeleton-query-wizard.ts

@@ -7,7 +7,7 @@ import { QueryLanguage } from "./common/query-language";
 import { askForLanguage, isFolderAlreadyInWorkspace } from "./helpers";
 import { getErrorMessage } from "./pure/helpers-pure";
 import { QlPackGenerator } from "./qlpack-generator";
-import { DatabaseManager } from "./local-databases";
+import { DatabaseItem, DatabaseManager } from "./local-databases";
 import * as databaseFetcher from "./databaseFetcher";
 import { ProgressCallback, UserCancellationException } from "./progress";
 
@@ -239,20 +239,20 @@ export class SkeletonQueryWizard {
 
     const databaseNwo = QUERY_LANGUAGE_TO_DATABASE_REPO[this.language];
 
-    // Check that we haven't already downloaded a database for this language
-    const existingDatabaseItem = await this.databaseManager.digForDatabaseItem(
+    const existingDatabaseItem = await this.findDatabaseItemByNwo(
       this.language,
       databaseNwo,
+      this.databaseManager.databaseItems,
     );
 
     if (existingDatabaseItem) {
       // select the found database
       await this.databaseManager.setCurrentDatabaseItem(existingDatabaseItem);
     } else {
-      const sameLanguageDatabaseItem =
-        await this.databaseManager.digForDatabaseWithSameLanguage(
-          this.language,
-        );
+      const sameLanguageDatabaseItem = await this.findDatabaseItemByLanguage(
+        this.language,
+        this.databaseManager.databaseItems,
+      );
 
       if (sameLanguageDatabaseItem) {
         // select the found database
@@ -265,4 +265,31 @@ export class SkeletonQueryWizard {
       }
     }
   }
+
+  public async findDatabaseItemByNwo(
+    language: string,
+    databaseNwo: string,
+    databaseItems: readonly DatabaseItem[],
+  ): Promise<DatabaseItem | undefined> {
+    const dbItems = databaseItems || [];
+    const dbs = dbItems.filter(
+      (db) => db.language === language && db.name === databaseNwo,
+    );
+    if (dbs.length === 0) {
+      return undefined;
+    }
+    return dbs[0];
+  }
+
+  public async findDatabaseItemByLanguage(
+    language: string,
+    databaseItems: readonly DatabaseItem[],
+  ): Promise<DatabaseItem | undefined> {
+    const dbItems = databaseItems || [];
+    const dbs = dbItems.filter((db) => db.language === language);
+    if (dbs.length === 0) {
+      return undefined;
+    }
+    return dbs[0];
+  }
 }

extensions/ql-vscode/test/factories/databases/databases.ts

@@ -0,0 +1,46 @@
+import { join } from "path";
+import { Uri } from "vscode";
+import {
+  DatabaseContents,
+  DatabaseItemImpl,
+  FullDatabaseOptions,
+} from "../../../src/local-databases";
+import { DirResult } from "tmp";
+
+export function mockDbOptions(): FullDatabaseOptions {
+  return {
+    dateAdded: 123,
+    ignoreSourceArchive: false,
+    language: "",
+  };
+}
+
+export function createMockDB(
+  dir: DirResult,
+  dbOptions = mockDbOptions(),
+  // source archive location must be a real(-ish) location since
+  // tests will add this to the workspace location
+  sourceArchiveUri?: Uri,
+  databaseUri?: Uri,
+): DatabaseItemImpl {
+  sourceArchiveUri = sourceArchiveUri || sourceLocationUri(dir);
+  databaseUri = databaseUri || dbLocationUri(dir);
+
+  return new DatabaseItemImpl(
+    databaseUri,
+    {
+      sourceArchiveUri,
+      datasetUri: databaseUri,
+    } as DatabaseContents,
+    dbOptions,
+    () => void 0,
+  );
+}
+
+export function sourceLocationUri(dir: DirResult) {
+  return Uri.file(join(dir.name, "src.zip"));
+}
+
+export function dbLocationUri(dir: DirResult) {
+  return Uri.file(join(dir.name, "db"));
+}

extensions/ql-vscode/test/vscode-tests/cli-integration/skeleton-query-wizard.test.ts

@@ -13,13 +13,20 @@ import { createFileSync, ensureDirSync, removeSync } from "fs-extra";
 import { join } from "path";
 import { CancellationTokenSource } from "vscode-jsonrpc";
 import { testCredentialsWithStub } from "../../factories/authentication";
-import { DatabaseItem, DatabaseManager } from "../../../src/local-databases";
+import {
+  DatabaseItem,
+  DatabaseManager,
+  FullDatabaseOptions,
+} from "../../../src/local-databases";
 import * as databaseFetcher from "../../../src/databaseFetcher";
+import { createMockDB } from "../../factories/databases/databases";
 
 jest.setTimeout(40_000);
 
 describe("SkeletonQueryWizard", () => {
+  let mockCli: CodeQLCliServer;
   let wizard: SkeletonQueryWizard;
+  let mockDatabaseManager: DatabaseManager;
   let dir: tmp.DirResult;
   let storagePath: string;
   let quickPickSpy: jest.SpiedFunction<typeof window.showQuickPick>;
@@ -42,29 +49,30 @@ describe("SkeletonQueryWizard", () => {
   const token = new CancellationTokenSource().token;
   const credentials = testCredentialsWithStub();
   const chosenLanguage = "ruby";
-  const mockDatabaseManager = mockedObject<DatabaseManager>({
-    setCurrentDatabaseItem: jest.fn(),
-    digForDatabaseItem: jest.fn(),
-    digForDatabaseWithSameLanguage: jest.fn(),
-  });
-  const mockCli = mockedObject<CodeQLCliServer>({
-    resolveLanguages: jest
-      .fn()
-      .mockResolvedValue([
-        "ruby",
-        "javascript",
-        "go",
-        "java",
-        "python",
-        "csharp",
-        "cpp",
-      ]),
-    getSupportedLanguages: jest.fn(),
-  });
 
   jest.spyOn(extLogger, "log").mockResolvedValue(undefined);
 
   beforeEach(async () => {
+    mockCli = mockedObject<CodeQLCliServer>({
+      resolveLanguages: jest
+        .fn()
+        .mockResolvedValue([
+          "ruby",
+          "javascript",
+          "go",
+          "java",
+          "python",
+          "csharp",
+          "cpp",
+        ]),
+      getSupportedLanguages: jest.fn(),
+    });
+
+    mockDatabaseManager = mockedObject<DatabaseManager>({
+      setCurrentDatabaseItem: jest.fn(),
+      databaseItems: [] as DatabaseItem[],
+    });
+
     dir = tmp.dirSync({
       prefix: "skeleton_query_wizard_",
       unsafeCleanup: true,
@@ -214,6 +222,7 @@ describe("SkeletonQueryWizard", () => {
     describe("if database is also already downloaded", () => {
       let databaseNwo: string;
       let databaseItem: DatabaseItem;
+      let mockDatabaseManagerWithItems: DatabaseManager;
 
       beforeEach(async () => {
         databaseNwo = QUERY_LANGUAGE_TO_DATABASE_REPO[chosenLanguage];
@@ -223,9 +232,19 @@ describe("SkeletonQueryWizard", () => {
           language: chosenLanguage,
         } as DatabaseItem;
 
-        jest
-          .spyOn(mockDatabaseManager, "digForDatabaseItem")
-          .mockResolvedValue([databaseItem] as any);
+        mockDatabaseManagerWithItems = mockedObject<DatabaseManager>({
+          setCurrentDatabaseItem: jest.fn(),
+          databaseItems: [databaseItem] as DatabaseItem[],
+        });
+
+        wizard = new SkeletonQueryWizard(
+          mockCli,
+          jest.fn(),
+          credentials,
+          extLogger,
+          mockDatabaseManagerWithItems,
+          token,
+        );
       });
 
       it("should not download a new database for language", async () => {
@@ -237,9 +256,9 @@ describe("SkeletonQueryWizard", () => {
       it("should select an existing database", async () => {
         await wizard.execute();
 
-        expect(mockDatabaseManager.setCurrentDatabaseItem).toHaveBeenCalledWith(
-          [databaseItem],
-        );
+        expect(
+          mockDatabaseManagerWithItems.setCurrentDatabaseItem,
+        ).toHaveBeenCalledWith(databaseItem);
       });
 
       it("should open the new query file", async () => {
@@ -254,12 +273,6 @@ describe("SkeletonQueryWizard", () => {
     });
 
     describe("if database is missing", () => {
-      beforeEach(async () => {
-        jest
-          .spyOn(mockDatabaseManager, "digForDatabaseItem")
-          .mockResolvedValue(undefined);
-      });
-
       describe("if the user choses to downloaded the suggested database from GitHub", () => {
         it("should download a new database for language", async () => {
           await wizard.execute();
@@ -335,4 +348,71 @@ describe("SkeletonQueryWizard", () => {
       });
     });
   });
+
+  describe("findDatabaseItemByNwo", () => {
+    describe("when the item exists", () => {
+      it("should return the database item", async () => {
+        const mockDbItem = createMockDB(dir);
+        const mockDbItem2 = createMockDB(dir);
+
+        const databaseItem = await wizard.findDatabaseItemByNwo(
+          mockDbItem.language,
+          mockDbItem.name,
+          [mockDbItem, mockDbItem2],
+        );
+
+        expect(databaseItem!.language).toEqual(mockDbItem.language);
+        expect(databaseItem!.name).toEqual(mockDbItem.name);
+      });
+    });
+
+    describe("when the item doesn't exist", () => {
+      it("should return nothing", async () => {
+        const mockDbItem = createMockDB(dir);
+        const mockDbItem2 = createMockDB(dir);
+
+        const databaseItem = await wizard.findDatabaseItemByNwo(
+          "ruby",
+          "mock-nwo",
+          [mockDbItem, mockDbItem2],
+        );
+
+        expect(databaseItem).toBeUndefined();
+      });
+    });
+  });
+
+  describe("findDatabaseItemByLanguage", () => {
+    describe("when the item exists", () => {
+      it("should return the database item", async () => {
+        const mockDbItem = createMockDB(dir, {
+          language: "ruby",
+        } as FullDatabaseOptions);
+        const mockDbItem2 = createMockDB(dir, {
+          language: "javascript",
+        } as FullDatabaseOptions);
+
+        const databaseItem = await wizard.findDatabaseItemByLanguage("ruby", [
+          mockDbItem,
+          mockDbItem2,
+        ]);
+
+        expect(databaseItem).toEqual(mockDbItem);
+      });
+    });
+
+    describe("when the item doesn't exist", () => {
+      it("should return nothing", async () => {
+        const mockDbItem = createMockDB(dir);
+        const mockDbItem2 = createMockDB(dir);
+
+        const databaseItem = await wizard.findDatabaseItemByLanguage("ruby", [
+          mockDbItem,
+          mockDbItem2,
+        ]);
+
+        expect(databaseItem).toBeUndefined();
+      });
+    });
+  });
 });