Added logic to clean expanded state when it's updated (#1938)

extensions/ql-vscode/src/databases/db-item-expansion.ts

@@ -1,4 +1,4 @@
-import { DbItem, DbItemKind } from "./db-item";
+import { DbItem, DbItemKind, flattenDbItems } from "./db-item";
 
 export type ExpandedDbItem =
   | RootLocalExpandedDbItem
@@ -68,6 +68,16 @@ export function replaceExpandedItem(
   return newExpandedItems;
 }
 
+export function cleanNonExistentExpandedItems(
+  currentExpandedItems: ExpandedDbItem[],
+  dbItems: DbItem[],
+): ExpandedDbItem[] {
+  const flattenedDbItems = flattenDbItems(dbItems);
+  return currentExpandedItems.filter((i) =>
+    flattenedDbItems.some((dbItem) => isDbItemEqualToExpandedDbItem(dbItem, i)),
+  );
+}
+
 function mapDbItemToExpandedDbItem(dbItem: DbItem): ExpandedDbItem {
   switch (dbItem.kind) {
     case DbItemKind.RootLocal:

extensions/ql-vscode/src/databases/db-item.ts

@@ -149,3 +149,32 @@ const SelectableDbItemKinds = [
   DbItemKind.RemoteOwner,
   DbItemKind.RemoteRepo,
 ];
+
+export function flattenDbItems(dbItems: DbItem[]): DbItem[] {
+  const allItems: DbItem[] = [];
+
+  for (const dbItem of dbItems) {
+    allItems.push(dbItem);
+    switch (dbItem.kind) {
+      case DbItemKind.RootLocal:
+        allItems.push(...flattenDbItems(dbItem.children));
+        break;
+      case DbItemKind.LocalList:
+        allItems.push(...flattenDbItems(dbItem.databases));
+        break;
+      case DbItemKind.RootRemote:
+        allItems.push(...flattenDbItems(dbItem.children));
+        break;
+      case DbItemKind.RemoteUserDefinedList:
+        allItems.push(...dbItem.repos);
+        break;
+      case DbItemKind.LocalDatabase:
+      case DbItemKind.RemoteSystemDefinedList:
+      case DbItemKind.RemoteOwner:
+      case DbItemKind.RemoteRepo:
+        break;
+    }
+  }
+
+  return allItems;
+}

extensions/ql-vscode/src/databases/db-manager.ts

@@ -14,6 +14,7 @@ import {
   updateExpandedItem,
   replaceExpandedItem,
   ExpandedDbItem,
+  cleanNonExistentExpandedItems,
 } from "./db-item-expansion";
 import {
   getSelectedDbItem,
@@ -86,7 +87,7 @@ export class DbManager {
       itemExpanded,
     );
 
-    await this.setExpandedItems(newExpandedItems);
+    await this.updateExpandedItems(newExpandedItems);
   }
 
   public async addNewRemoteRepo(
@@ -133,7 +134,7 @@ export class DbManager {
       newDbItem,
     );
 
-    await this.setExpandedItems(newExpandedItems);
+    await this.updateExpandedItems(newExpandedItems);
   }
 
   public async renameLocalDb(
@@ -184,4 +185,25 @@ export class DbManager {
       items,
     );
   }
+
+  private async updateExpandedItems(items: ExpandedDbItem[]): Promise<void> {
+    let itemsToStore;
+
+    const dbItemsResult = this.getDbItems();
+
+    if (dbItemsResult.isFailure) {
+      // Log an error but don't throw an exception since if the db items are failing
+      // to be read, then there is a bigger problem than the expanded state.
+      void this.app.logger.log(
+        `Could not read db items when calculating expanded state: ${JSON.stringify(
+          dbItemsResult.errors,
+        )}`,
+      );
+      itemsToStore = items;
+    } else {
+      itemsToStore = cleanNonExistentExpandedItems(items, dbItemsResult.value);
+    }
+
+    await this.setExpandedItems(itemsToStore);
+  }
 }

extensions/ql-vscode/test/unit-tests/databases/db-item-expansion.test.ts

@@ -3,9 +3,12 @@ import {
   ExpandedDbItem,
   ExpandedDbItemKind,
   replaceExpandedItem,
+  cleanNonExistentExpandedItems,
 } from "../../../src/databases/db-item-expansion";
 import {
+  createLocalListDbItem,
   createRemoteUserDefinedListDbItem,
+  createRootLocalDbItem,
   createRootRemoteDbItem,
 } from "../../factories/db-item-factories";
 
@@ -157,4 +160,62 @@ describe("db item expansion", () => {
       ]);
     });
   });
+
+  describe("cleanNonExistentExpandedItems", () => {
+    it("should remove non-existent items", () => {
+      const currentExpandedItems: ExpandedDbItem[] = [
+        {
+          kind: ExpandedDbItemKind.RootRemote,
+        },
+        {
+          kind: ExpandedDbItemKind.RemoteUserDefinedList,
+          listName: "list1",
+        },
+        {
+          kind: ExpandedDbItemKind.RemoteUserDefinedList,
+          listName: "list2",
+        },
+        {
+          kind: ExpandedDbItemKind.LocalUserDefinedList,
+          listName: "list1",
+        },
+      ];
+
+      const dbItems = [
+        createRootRemoteDbItem({
+          children: [
+            createRemoteUserDefinedListDbItem({
+              listName: "list2",
+            }),
+          ],
+        }),
+        createRootLocalDbItem({
+          children: [
+            createLocalListDbItem({
+              listName: "list1",
+            }),
+          ],
+        }),
+      ];
+
+      const newExpandedItems = cleanNonExistentExpandedItems(
+        currentExpandedItems,
+        dbItems,
+      );
+
+      expect(newExpandedItems).toEqual([
+        {
+          kind: ExpandedDbItemKind.RootRemote,
+        },
+        {
+          kind: ExpandedDbItemKind.RemoteUserDefinedList,
+          listName: "list2",
+        },
+        {
+          kind: ExpandedDbItemKind.LocalUserDefinedList,
+          listName: "list1",
+        },
+      ]);
+    });
+  });
 });

extensions/ql-vscode/test/unit-tests/databases/db-item.test.ts

@@ -0,0 +1,144 @@
+import {
+  DbItem,
+  DbItemKind,
+  flattenDbItems,
+} from "../../../src/databases/db-item";
+import {
+  createLocalDatabaseDbItem,
+  createLocalListDbItem,
+  createRemoteOwnerDbItem,
+  createRemoteRepoDbItem,
+  createRemoteSystemDefinedListDbItem,
+  createRemoteUserDefinedListDbItem,
+  createRootLocalDbItem,
+  createRootRemoteDbItem,
+} from "../../factories/db-item-factories";
+
+describe("DbItem", () => {
+  describe("flattenDbItems", () => {
+    it("should flatten a list of DbItems", () => {
+      const dbItems = [
+        createRootRemoteDbItem({
+          children: [
+            createRemoteSystemDefinedListDbItem({ listName: "top10" }),
+            createRemoteSystemDefinedListDbItem({ listName: "top100" }),
+            createRemoteUserDefinedListDbItem({
+              listName: "remote-list1",
+              repos: [
+                createRemoteRepoDbItem({ repoFullName: "owner1/repo1" }),
+                createRemoteRepoDbItem({ repoFullName: "owner1/repo2" }),
+              ],
+            }),
+            createRemoteUserDefinedListDbItem({
+              listName: "remote-list2",
+              repos: [
+                createRemoteRepoDbItem({ repoFullName: "owner2/repo1" }),
+                createRemoteRepoDbItem({ repoFullName: "owner2/repo2" }),
+              ],
+            }),
+            createRemoteOwnerDbItem({ ownerName: "owner1" }),
+            createRemoteRepoDbItem({ repoFullName: "owner3/repo3" }),
+          ],
+        }),
+        createRootLocalDbItem({
+          children: [
+            createLocalListDbItem({
+              listName: "local-list1",
+              databases: [
+                createLocalDatabaseDbItem({ databaseName: "local-db1" }),
+              ],
+            }),
+            createLocalDatabaseDbItem({ databaseName: "local-db2" }),
+          ],
+        }),
+      ];
+
+      const flattenedItems = flattenDbItems(dbItems);
+
+      expect(flattenedItems.length).toEqual(15);
+      checkRootRemoteExists(flattenedItems);
+      checkSystemDefinedListExists(flattenedItems, "top10");
+      checkSystemDefinedListExists(flattenedItems, "top100");
+      checkUserDefinedListExists(flattenedItems, "remote-list1");
+      checkRemoteRepoExists(flattenedItems, "owner1/repo1");
+      checkRemoteRepoExists(flattenedItems, "owner1/repo2");
+      checkRemoteRepoExists(flattenedItems, "owner2/repo1");
+      checkRemoteRepoExists(flattenedItems, "owner2/repo2");
+      checkRemoteOwnerExists(flattenedItems, "owner1");
+      checkRemoteRepoExists(flattenedItems, "owner3/repo3");
+      checkRootLocalExists(flattenedItems);
+      checkLocalListExists(flattenedItems, "local-list1");
+      checkLocalDbExists(flattenedItems, "local-db1");
+      checkLocalDbExists(flattenedItems, "local-db2");
+    });
+
+    function checkRootRemoteExists(items: DbItem[]): void {
+      expect(
+        items.find((item) => item.kind === DbItemKind.RootRemote),
+      ).toBeDefined();
+    }
+
+    function checkUserDefinedListExists(items: DbItem[], name: string): void {
+      expect(
+        items.find(
+          (item) =>
+            item.kind === DbItemKind.RemoteUserDefinedList &&
+            item.listName === name,
+        ),
+      ).toBeDefined();
+    }
+
+    function checkSystemDefinedListExists(items: DbItem[], name: string): void {
+      expect(
+        items.find(
+          (item) =>
+            item.kind === DbItemKind.RemoteSystemDefinedList &&
+            item.listName === name,
+        ),
+      ).toBeDefined();
+    }
+
+    function checkRemoteOwnerExists(items: DbItem[], name: string): void {
+      expect(
+        items.find(
+          (item) =>
+            item.kind === DbItemKind.RemoteOwner && item.ownerName === name,
+        ),
+      ).toBeDefined();
+    }
+
+    function checkRemoteRepoExists(items: DbItem[], name: string): void {
+      expect(
+        items.find(
+          (item) =>
+            item.kind === DbItemKind.RemoteRepo && item.repoFullName === name,
+        ),
+      ).toBeDefined();
+    }
+
+    function checkRootLocalExists(items: DbItem[]): void {
+      expect(
+        items.find((item) => item.kind === DbItemKind.RootLocal),
+      ).toBeDefined();
+    }
+
+    function checkLocalListExists(items: DbItem[], name: string): void {
+      expect(
+        items.find(
+          (item) =>
+            item.kind === DbItemKind.LocalList && item.listName === name,
+        ),
+      ).toBeDefined();
+    }
+
+    function checkLocalDbExists(items: DbItem[], name: string): void {
+      expect(
+        items.find(
+          (item) =>
+            item.kind === DbItemKind.LocalDatabase &&
+            item.databaseName === name,
+        ),
+      ).toBeDefined();
+    }
+  });
+});