diff --git a/extensions/ql-vscode/src/databases/config/db-config-store.ts b/extensions/ql-vscode/src/databases/config/db-config-store.ts index 18b3f66ec..6807dcf4f 100644 --- a/extensions/ql-vscode/src/databases/config/db-config-store.ts +++ b/extensions/ql-vscode/src/databases/config/db-config-store.ts @@ -9,7 +9,6 @@ import { SelectedDbItem, SelectedDbItemKind, } from "./db-config"; -import * as lodash from "lodash"; import * as chokidar from "chokidar"; import { DisposableObject, DisposeHandler } from "../../pure/disposable-object"; import { DbConfigValidator } from "./db-config-validator"; @@ -27,7 +26,10 @@ import { DbItem, DbItemKind, } from "../db-item"; -import { mapDbItemToSelectedDbItem } from "../db-item-selection"; +import { + compareSelectedKindIsEqual, + mapDbItemToSelectedDbItem, +} from "../db-item-selection"; export class DbConfigStore extends DisposableObject { public readonly onDidChangeConfig: AppEvent; @@ -98,7 +100,7 @@ export class DbConfigStore extends DisposableObject { throw Error("Cannot remove item if config is not loaded"); } - const config: DbConfig = cloneDbConfig(this.config); + const config = cloneDbConfig(this.config); const selectedItem: SelectedDbItem | undefined = config.selected; // Remove item from databases @@ -115,7 +117,7 @@ export class DbConfigStore extends DisposableObject { ); break; case DbItemKind.LocalDatabase: - // TODO: Remove databases from Disk once implemented + // When we start using local databases these need to be removed from disk as well. if (dbItem.parentListName) { const parent = config.databases.local.lists.find( (list) => list.name === dbItem.parentListName, @@ -162,12 +164,11 @@ export class DbConfigStore extends DisposableObject { // Remove item from selected const removedItem = mapDbItemToSelectedDbItem(dbItem); - if (selectedItem) { + if (selectedItem && removedItem) { // if removedItem has a parentList, check if parentList is selectedItem if ( - removedItem && - (removedItem.kind === SelectedDbItemKind.LocalUserDefinedList || - removedItem.kind === SelectedDbItemKind.RemoteUserDefinedList) + removedItem.kind === SelectedDbItemKind.LocalUserDefinedList || + removedItem.kind === SelectedDbItemKind.RemoteUserDefinedList ) { if ( (selectedItem.kind === SelectedDbItemKind.LocalDatabase || @@ -176,7 +177,8 @@ export class DbConfigStore extends DisposableObject { ) { config.selected = undefined; } - } else if (lodash.isEqual(removedItem, selectedItem)) { + } + if (compareSelectedKindIsEqual(removedItem, selectedItem)) { config.selected = undefined; } } diff --git a/extensions/ql-vscode/src/databases/db-item-selection.ts b/extensions/ql-vscode/src/databases/db-item-selection.ts index 4ae9f9f90..495d730a4 100644 --- a/extensions/ql-vscode/src/databases/db-item-selection.ts +++ b/extensions/ql-vscode/src/databases/db-item-selection.ts @@ -1,5 +1,12 @@ import { DbItem, DbItemKind, LocalDbItem, RemoteDbItem } from "./db-item"; -import { SelectedDbItem, SelectedDbItemKind } from "./config/db-config"; +import { + SelectedDbItem, + SelectedDbItemKind, + SelectedLocalDatabase, + SelectedLocalUserDefinedList, + SelectedRemoteOwner, + SelectedRemoteRepository, +} from "./config/db-config"; export function getSelectedDbItem(dbItems: DbItem[]): DbItem | undefined { for (const dbItem of dbItems) { @@ -92,3 +99,39 @@ export function mapDbItemToSelectedDbItem( }; } } + +export function compareSelectedKindIsEqual( + item1: SelectedDbItem, + item2: SelectedDbItem, +): boolean { + if (item1.kind === item2.kind) { + switch (item1.kind) { + case SelectedDbItemKind.LocalUserDefinedList: + case SelectedDbItemKind.RemoteUserDefinedList: + case SelectedDbItemKind.RemoteSystemDefinedList: + return ( + item1.listName === (item2 as SelectedLocalUserDefinedList).listName + ); + case SelectedDbItemKind.RemoteOwner: + return item1.ownerName === (item2 as SelectedRemoteOwner).ownerName; + case SelectedDbItemKind.LocalDatabase: { + const selectedItem = item2 as SelectedLocalDatabase; + return ( + item1.databaseName === selectedItem.databaseName && + item1.listName === selectedItem.listName + ); + } + case SelectedDbItemKind.RemoteRepository: { + const selectedItem = item2 as SelectedRemoteRepository; + return ( + item1.repositoryName === selectedItem.repositoryName && + item1.listName === selectedItem.listName + ); + } + default: + return false; + } + } else { + return false; + } +} diff --git a/extensions/ql-vscode/src/databases/db-manager.ts b/extensions/ql-vscode/src/databases/db-manager.ts index 02b57a769..88f481b8e 100644 --- a/extensions/ql-vscode/src/databases/db-manager.ts +++ b/extensions/ql-vscode/src/databases/db-manager.ts @@ -77,6 +77,10 @@ export class DbManager { public async removeDbItem(dbItem: DbItem): Promise { await this.dbConfigStore.removeDbItem(dbItem); + + // Updating the expanded items takes care of cleaning up + // any non-existent items. + await this.updateExpandedItems(this.getExpandedItems()); } public async updateDbItemExpandedState( 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 index 3af6958f1..6f290a021 100644 --- 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 @@ -13,6 +13,8 @@ import { import { createLocalDatabaseDbItem, createLocalListDbItem, + createRemoteOwnerDbItem, + createRemoteRepoDbItem, createRemoteUserDefinedListDbItem, } from "../../../factories/db-item-factories"; import { createMockApp } from "../../../__mocks__/appMock"; @@ -365,4 +367,133 @@ describe("db config store", () => { configStore.dispose(); }); }); + + describe("db and list deletion", () => { + let app: App; + let configPath: string; + + beforeEach(async () => { + app = createMockApp({ + extensionPath, + workspaceStoragePath: tempWorkspaceStoragePath, + }); + + configPath = join(tempWorkspaceStoragePath, "workspace-databases.json"); + }); + + it("should remove a single db item", async () => { + // Initial set up + const dbConfig = createDbConfig({ + remoteOwners: ["owner1", "owner2"], + selected: { + kind: SelectedDbItemKind.RemoteOwner, + ownerName: "owner1", + }, + }); + + await writeJSON(configPath, dbConfig); + + const configStore = new DbConfigStore(app); + await configStore.initialize(); + + // Remove + const currentDbItem = createRemoteOwnerDbItem({ + ownerName: "owner1", + }); + await configStore.removeDbItem(currentDbItem); + + // Read the config file + const updatedDbConfig = (await readJSON(configPath)) as DbConfig; + + // Check that the config file has been updated + const updatedRemoteDbs = updatedDbConfig.databases.remote; + expect(updatedRemoteDbs.owners).toHaveLength(1); + expect(updatedRemoteDbs.owners[0]).toEqual("owner2"); + + expect(updatedDbConfig.selected).toEqual(undefined); + + configStore.dispose(); + }); + + it("should remove a list db item", async () => { + // Initial set up + const dbConfig = createDbConfig({ + remoteLists: [ + { + name: "list1", + repositories: ["owner/repo1", "owner/repo2"], + }, + ], + selected: { + kind: SelectedDbItemKind.RemoteUserDefinedList, + listName: "list1", + }, + }); + + await writeJSON(configPath, dbConfig); + + const configStore = new DbConfigStore(app); + await configStore.initialize(); + + // Remove + const currentDbItem = createRemoteUserDefinedListDbItem({ + listName: "list1", + }); + await configStore.removeDbItem(currentDbItem); + + // Read the config file + const updatedDbConfig = (await readJSON(configPath)) as DbConfig; + + // Check that the config file has been updated + const updatedRemoteDbs = updatedDbConfig.databases.remote; + expect(updatedRemoteDbs.repositoryLists).toHaveLength(0); + + expect(updatedDbConfig.selected).toEqual(undefined); + + configStore.dispose(); + }); + + it("should remove a db item in a list", async () => { + // Initial set up + const dbConfig = createDbConfig({ + remoteLists: [ + { + name: "list1", + repositories: ["owner/repo1", "owner/repo2"], + }, + ], + selected: { + kind: SelectedDbItemKind.RemoteRepository, + repositoryName: "owner/repo1", + listName: "list1", + }, + }); + + await writeJSON(configPath, dbConfig); + + const configStore = new DbConfigStore(app); + await configStore.initialize(); + + // Remove + const currentDbItem = createRemoteRepoDbItem({ + repoFullName: "owner/repo1", + parentListName: "list1", + }); + await configStore.removeDbItem(currentDbItem); + + // Read the config file + const updatedDbConfig = (await readJSON(configPath)) as DbConfig; + + // Check that the config file has been updated + const updatedRemoteDbs = updatedDbConfig.databases.remote; + expect(updatedRemoteDbs.repositoryLists[0].repositories).toHaveLength(1); + expect(updatedRemoteDbs.repositoryLists[0].repositories[0]).toEqual( + "owner/repo2", + ); + + expect(updatedDbConfig.selected).toEqual(undefined); + + configStore.dispose(); + }); + }); });