Merge pull request #2507 from github/nora/remove-1000-list-limit

norascheuch · web-flow · commit a03b3dca86a6 · 2023-06-13T09:57:16.000+02:00
Remove limit of 1000 repos per list
diff --git a/extensions/ql-vscode/src/databases/config/db-config-store.ts b/extensions/ql-vscode/src/databases/config/db-config-store.ts
@@ -147,14 +147,10 @@ export class DbConfigStore extends DisposableObject {
     await this.writeConfig(config);
   }
 
-  /**
-   * Adds a list of remote repositories to an existing repository list and removes duplicates.
-   * @returns a list of repositories that were not added because the list reached 1000 entries.
-   */
   public async addRemoteReposToList(
     repoNwoList: string[],
     parentList: string,
-  ): Promise<string[]> {
+  ): Promise<void> {
     if (!this.config) {
       throw Error("Cannot add variant analysis repos if config is not loaded");
     }
@@ -172,21 +168,15 @@ export class DbConfigStore extends DisposableObject {
       ...new Set([...parent.repositories, ...repoNwoList]),
     ];
 
-    parent.repositories = newRepositoriesList.slice(0, 1000);
-    const truncatedRepositories = newRepositoriesList.slice(1000);
+    parent.repositories = newRepositoriesList;
 
     await this.writeConfig(config);
-    return truncatedRepositories;
   }
 
-  /**
-   * Adds one remote repository
-   * @returns either nothing, or, if a parentList is given AND the number of repos on that list reaches 1000 returns the repo that was not added.
-   */
   public async addRemoteRepo(
     repoNwo: string,
     parentList?: string,
-  ): Promise<string[]> {
+  ): Promise<void> {
     if (!this.config) {
       throw Error("Cannot add variant analysis repo if config is not loaded");
     }
@@ -201,7 +191,6 @@ export class DbConfigStore extends DisposableObject {
       );
     }
 
-    const truncatedRepositories = [];
     const config = cloneDbConfig(this.config);
     if (parentList) {
       const parent = config.databases.variantAnalysis.repositoryLists.find(
@@ -210,15 +199,12 @@ export class DbConfigStore extends DisposableObject {
       if (!parent) {
         throw Error(`Cannot find parent list '${parentList}'`);
       } else {
-        const newRepositories = [...parent.repositories, repoNwo];
-        parent.repositories = newRepositories.slice(0, 1000);
-        truncatedRepositories.push(...newRepositories.slice(1000));
+        parent.repositories = [...parent.repositories, repoNwo];
       }
     } else {
       config.databases.variantAnalysis.repositories.push(repoNwo);
     }
     await this.writeConfig(config);
-    return truncatedRepositories;
   }
 
   public async addRemoteOwner(owner: string): Promise<void> {
diff --git a/extensions/ql-vscode/src/databases/db-manager.ts b/extensions/ql-vscode/src/databases/db-manager.ts
@@ -101,15 +101,15 @@ export class DbManager extends DisposableObject {
   public async addNewRemoteRepo(
     nwo: string,
     parentList?: string,
-  ): Promise<string[]> {
-    return await this.dbConfigStore.addRemoteRepo(nwo, parentList);
+  ): Promise<void> {
+    await this.dbConfigStore.addRemoteRepo(nwo, parentList);
   }
 
   public async addNewRemoteReposToList(
     nwoList: string[],
     parentList: string,
-  ): Promise<string[]> {
-    return await this.dbConfigStore.addRemoteReposToList(nwoList, parentList);
+  ): Promise<void> {
+    await this.dbConfigStore.addRemoteReposToList(nwoList, parentList);
   }
 
   public async addNewRemoteOwner(owner: string): Promise<void> {
diff --git a/extensions/ql-vscode/src/databases/ui/db-panel.ts b/extensions/ql-vscode/src/databases/ui/db-panel.ts
@@ -183,14 +183,7 @@ export class DbPanel extends DisposableObject {
       return;
     }
 
-    const truncatedRepositories = await this.dbManager.addNewRemoteRepo(
-      nwo,
-      parentList,
-    );
-
-    if (parentList) {
-      this.reportAnyTruncatedRepos(truncatedRepositories, parentList);
-    }
+    await this.dbManager.addNewRemoteRepo(nwo, parentList);
   }
 
   private async addNewRemoteOwner(): Promise<void> {
@@ -415,26 +408,11 @@ export class DbPanel extends DisposableObject {
 
         progress.report({ increment: 10, message: "Processing results..." });
 
-        const truncatedRepositories =
-          await this.dbManager.addNewRemoteReposToList(repositories, listName);
-        this.reportAnyTruncatedRepos(truncatedRepositories, listName);
+        await this.dbManager.addNewRemoteReposToList(repositories, listName);
       },
     );
   }
 
-  private reportAnyTruncatedRepos(
-    truncatedRepositories: string[],
-    listName: string,
-  ) {
-    if (truncatedRepositories.length > 0) {
-      void showAndLogErrorMessage(
-        `Some repositories were not added to '${listName}' because a list can only have 1000 entries. Excluded repositories: ${truncatedRepositories.join(
-          ", ",
-        )}`,
-      );
-    }
-  }
-
   private async onDidCollapseElement(
     event: TreeViewExpansionEvent<DbTreeViewItem>,
   ): Promise<void> {
diff --git a/extensions/ql-vscode/test/unit-tests/databases/config/db-config-store.test.ts b/extensions/ql-vscode/test/unit-tests/databases/config/db-config-store.test.ts
@@ -262,7 +262,7 @@ describe("db config store", () => {
       });
 
       // Add
-      const response = await configStore.addRemoteReposToList(
+      await configStore.addRemoteReposToList(
         ["owner/repo1", "owner/repo2"],
         "list1",
       );
@@ -278,72 +278,6 @@ describe("db config store", () => {
         name: "list1",
         repositories: ["owner/repo1", "owner/repo2"],
       });
-      expect(response).toEqual([]);
-
-      configStore.dispose();
-    });
-
-    it("should add no more than 1000 repositories to a remote list when adding multiple repos", async () => {
-      // Initial set up
-      const dbConfig = createDbConfig({
-        remoteLists: [
-          {
-            name: "list1",
-            repositories: [],
-          },
-        ],
-      });
-
-      const configStore = await initializeConfig(dbConfig, configPath, app);
-
-      // Add
-      const response = await configStore.addRemoteReposToList(
-        [...Array(1001).keys()].map((i) => `owner/db${i}`),
-        "list1",
-      );
-
-      // Read the config file
-      const updatedDbConfig = (await readJSON(configPath)) as DbConfig;
-
-      // Check that the config file has been updated
-      const updatedRemoteDbs = updatedDbConfig.databases.variantAnalysis;
-      expect(updatedRemoteDbs.repositories).toHaveLength(0);
-      expect(updatedRemoteDbs.repositoryLists).toHaveLength(1);
-      expect(updatedRemoteDbs.repositoryLists[0].repositories).toHaveLength(
-        1000,
-      );
-      expect(response).toEqual(["owner/db1000"]);
-
-      configStore.dispose();
-    });
-
-    it("should add no more than 1000 repositories to a remote list when adding one repo", async () => {
-      // Initial set up
-      const dbConfig = createDbConfig({
-        remoteLists: [
-          {
-            name: "list1",
-            repositories: [...Array(1000).keys()].map((i) => `owner/db${i}`),
-          },
-        ],
-      });
-
-      const configStore = await initializeConfig(dbConfig, configPath, app);
-
-      // Add
-      const reponse = await configStore.addRemoteRepo("owner/db1000", "list1");
-
-      // Read the config file
-      const updatedDbConfig = (await readJSON(configPath)) as DbConfig;
-
-      // Check that the config file has been updated
-      const updatedRemoteDbs = updatedDbConfig.databases.variantAnalysis;
-      expect(updatedRemoteDbs.repositories).toHaveLength(0);
-      expect(updatedRemoteDbs.repositoryLists).toHaveLength(1);
-      expect(updatedRemoteDbs.repositoryLists[0].repositories).toHaveLength(
-        1000,
-      );
-      expect(reponse).toEqual(["owner/db1000"]);
 
       configStore.dispose();
     });
diff --git a/extensions/ql-vscode/test/unit-tests/databases/db-manager.test.ts b/extensions/ql-vscode/test/unit-tests/databases/db-manager.test.ts
@@ -115,46 +115,6 @@ describe("db manager", () => {
         });
       });
 
-      it("should return truncated repos when adding multiple repos to a user defined list", async () => {
-        const dbConfig: DbConfig = createDbConfig({
-          remoteLists: [
-            {
-              name: "my-list-1",
-              repositories: [...Array(1000).keys()].map((i) => `owner/db${i}`),
-            },
-          ],
-        });
-
-        await saveDbConfig(dbConfig);
-
-        const response = await dbManager.addNewRemoteReposToList(
-          ["owner2/repo2"],
-          "my-list-1",
-        );
-
-        expect(response).toEqual(["owner2/repo2"]);
-      });
-
-      it("should return truncated repos when adding one repo to a user defined list", async () => {
-        const dbConfig: DbConfig = createDbConfig({
-          remoteLists: [
-            {
-              name: "my-list-1",
-              repositories: [...Array(1000).keys()].map((i) => `owner/db${i}`),
-            },
-          ],
-        });
-
-        await saveDbConfig(dbConfig);
-
-        const response = await dbManager.addNewRemoteRepo(
-          "owner2/repo2",
-          "my-list-1",
-        );
-
-        expect(response).toEqual(["owner2/repo2"]);
-      });
-
       it("should add a new remote repo to a user defined list", async () => {
         const dbConfig: DbConfig = createDbConfig({
           remoteLists: [
