Turn off support for local dbs in new panel

This commit is contained in:
Charis Kyriakou
2023-01-20 08:39:18 +00:00
parent 8a6b361e84
commit 65623c07a2
13 changed files with 72 additions and 266 deletions

View File

@@ -48,120 +48,14 @@
},
"required": ["repositoryLists", "owners", "repositories"],
"additionalProperties": false
},
"local": {
"type": "object",
"properties": {
"lists": {
"type": "array",
"items": {
"type": "object",
"properties": {
"name": {
"type": "string"
},
"databases": {
"type": "array",
"items": {
"type": "object",
"properties": {
"name": {
"type": "string",
"minLength": 1
},
"dateAdded": {
"type": "number"
},
"language": {
"type": "string",
"minLength": 1
},
"storagePath": {
"type": "string",
"minLength": 1
}
},
"required": [
"name",
"dateAdded",
"language",
"storagePath"
],
"additionalProperties": false
}
}
},
"required": ["name", "databases"],
"additionalProperties": false
}
},
"databases": {
"type": "array",
"items": {
"type": "object",
"properties": {
"name": {
"type": "string",
"minLength": 1
},
"dateAdded": {
"type": "number"
},
"language": {
"type": "string",
"minLength": 1
},
"storagePath": {
"type": "string",
"minLength": 1
}
},
"required": ["name", "dateAdded", "language", "storagePath"],
"additionalProperties": false
}
}
},
"required": ["lists", "databases"],
"additionalProperties": false
}
},
"required": ["variantAnalysis", "local"],
"required": ["variantAnalysis"],
"additionalProperties": false
},
"selected": {
"type": "object",
"oneOf": [
{
"properties": {
"kind": {
"type": "string",
"enum": ["localUserDefinedList"]
},
"listName": {
"type": "string",
"minLength": 1
}
},
"required": ["kind", "listName"],
"additionalProperties": false
},
{
"properties": {
"kind": {
"type": "string",
"enum": ["localDatabase"]
},
"databaseName": {
"type": "string"
},
"listName": {
"type": "string",
"minLength": 1
}
},
"required": ["kind", "databaseName"],
"additionalProperties": false
},
{
"properties": {
"kind": {

View File

@@ -1,8 +1,10 @@
import { pathExists, outputJSON, readJSON, readJSONSync } from "fs-extra";
import { join } from "path";
import {
clearLocalDbConfig,
cloneDbConfig,
DbConfig,
initializeLocalDbConfig,
removeLocalDb,
removeLocalList,
removeRemoteList,
@@ -349,6 +351,7 @@ export class DbConfigStore extends DisposableObject {
}
private async writeConfig(config: DbConfig): Promise<void> {
clearLocalDbConfig(config);
await outputJSON(this.configPath, config, {
spaces: 2,
});
@@ -380,6 +383,7 @@ export class DbConfigStore extends DisposableObject {
}
if (newConfig) {
initializeLocalDbConfig(newConfig);
this.configErrors = this.configValidator.validate(newConfig);
}
@@ -414,6 +418,7 @@ export class DbConfigStore extends DisposableObject {
}
if (newConfig) {
initializeLocalDbConfig(newConfig);
this.configErrors = this.configValidator.validate(newConfig);
}

View File

@@ -1,7 +1,7 @@
import { readJsonSync } from "fs-extra";
import { resolve } from "path";
import Ajv from "ajv";
import { DbConfig } from "./db-config";
import { clearLocalDbConfig, DbConfig } from "./db-config";
import { findDuplicateStrings } from "../../text-utils";
import {
DbConfigValidationError,
@@ -18,6 +18,9 @@ export class DbConfigValidator {
public validate(dbConfig: DbConfig): DbConfigValidationError[] {
const ajv = new Ajv({ allErrors: true });
const localDbs = clearLocalDbConfig(dbConfig);
ajv.validate(this.schema, dbConfig);
if (ajv.errors) {
@@ -27,6 +30,13 @@ export class DbConfigValidator {
}));
}
// Add any local db config back so that we have a config
// object that respects its type and validation can happen
// as normal.
if (localDbs) {
dbConfig.databases.local = localDbs;
}
return [
...this.validateDbListNames(dbConfig),
...this.validateDbNames(dbConfig),

View File

@@ -325,6 +325,38 @@ export function removeRemoteOwner(
return config;
}
/**
* Removes local db config from a db config object, if one is set.
* We do this because we don't want to expose this feature to users
* yet (since it's only partially implemented), but we also don't want
* to remove all the code we've already implemented.
* @param config The config object to change.
* @returns Any removed local db config.
*/
export function clearLocalDbConfig(
config: DbConfig,
): LocalDbConfig | undefined {
let localDbs = undefined;
if (config && config.databases && config.databases.local) {
localDbs = config.databases.local;
delete (config.databases as any).local;
}
return localDbs;
}
/**
* Initializes the local db config, if the config object contains
* database configuration.
* @param config The config object to change.
*/
export function initializeLocalDbConfig(config: DbConfig): void {
if (config.databases) {
config.databases.local = { lists: [], databases: [] };
}
}
function cloneDbConfigSelectedItem(selected: SelectedDbItem): SelectedDbItem {
switch (selected.kind) {
case SelectedDbItemKind.LocalUserDefinedList:

View File

@@ -20,7 +20,7 @@ import {
getSelectedDbItem,
mapDbItemToSelectedDbItem,
} from "./db-item-selection";
import { createLocalTree, createRemoteTree } from "./db-tree-creator";
import { createRemoteTree } from "./db-tree-creator";
import { DbConfigValidationError } from "./db-validation-errors";
export class DbManager {
@@ -60,7 +60,6 @@ export class DbManager {
return ValueResult.ok([
createRemoteTree(configResult.value, expandedItems),
createLocalTree(configResult.value, expandedItems),
]);
}

View File

@@ -22,7 +22,6 @@ import {
DbListKind,
LocalDatabaseDbItem,
LocalListDbItem,
remoteDbKinds,
RemoteUserDefinedListDbItem,
} from "../db-item";
import { getDbItemName } from "../db-item-naming";
@@ -219,7 +218,7 @@ export class DbPanel extends DisposableObject {
}
private async addNewList(): Promise<void> {
const listKind = await this.getAddNewListKind();
const listKind = DbListKind.Remote;
const listName = await window.showInputBox({
prompt: "Enter a name for the new list",
@@ -237,47 +236,6 @@ export class DbPanel extends DisposableObject {
await this.dbManager.addNewList(listKind, listName);
}
private async getAddNewListKind(): Promise<DbListKind> {
const highlightedItem = await this.getHighlightedDbItem();
if (highlightedItem) {
return remoteDbKinds.includes(highlightedItem.kind)
? DbListKind.Remote
: DbListKind.Local;
} else {
const quickPickItems = [
{
label: "$(cloud) Variant Analysis",
detail: "Add a repository from GitHub",
alwaysShow: true,
kind: DbListKind.Remote,
},
{
label: "$(database) Local",
detail: "Import a database from the cloud or a local file",
alwaysShow: true,
kind: DbListKind.Local,
},
];
const selectedOption = await window.showQuickPick<AddListQuickPickItem>(
quickPickItems,
{
title: "Add a new database",
ignoreFocusOut: true,
},
);
if (!selectedOption) {
// We don't need to display a warning pop-up in this case, since the user just escaped out of the operation.
// We set 'true' to make this a silent exception.
throw new UserCancellationException(
"No database list kind selected",
true,
);
}
return selectedOption.kind;
}
}
private async setSelectedItem(treeViewItem: DbTreeViewItem): Promise<void> {
if (treeViewItem.dbItem === undefined) {
throw new Error(

View File

@@ -9,40 +9,6 @@
],
"owners": [],
"repositories": ["owner/repo1", "owner/repo2", "owner/repo3"]
},
"local": {
"lists": [
{
"name": "localList1",
"databases": [
{
"name": "foo/bar",
"dateAdded": 1668096745193,
"language": "go",
"storagePath": "/path/to/database/"
}
]
},
{
"name": "localList2",
"databases": [
{
"name": "foo/baz",
"dateAdded": 1668096760848,
"language": "javascript",
"storagePath": "/path/to/database/"
}
]
}
],
"databases": [
{
"name": "example-db",
"dateAdded": 1668096927267,
"language": "ruby",
"storagePath": "/path/to/database/"
}
]
}
},
"selected": {

View File

@@ -4,10 +4,6 @@
"repositoryLists": [],
"owners": [],
"repositories": []
},
"local": {
"lists": [],
"databases": []
}
}
}

View File

@@ -82,25 +82,6 @@ describe("db config store", () => {
"owner/repo2",
"owner/repo3",
]);
expect(config.databases.local.lists).toHaveLength(2);
expect(config.databases.local.lists[0]).toEqual({
name: "localList1",
databases: [
{
name: "foo/bar",
dateAdded: 1668096745193,
language: "go",
storagePath: "/path/to/database/",
},
],
});
expect(config.databases.local.databases).toHaveLength(1);
expect(config.databases.local.databases[0]).toEqual({
name: "example-db",
dateAdded: 1668096927267,
language: "ruby",
storagePath: "/path/to/database/",
});
expect(config.selected).toEqual({
kind: SelectedDbItemKind.VariantAnalysisUserDefinedList,
listName: "repoList1",
@@ -272,7 +253,7 @@ describe("db config store", () => {
configStore.dispose();
});
it("should add a local list", async () => {
it.skip("should add a local list", async () => {
// Initial set up
const dbConfig = createDbConfig();
@@ -370,7 +351,7 @@ describe("db config store", () => {
configStore.dispose();
});
it("should allow renaming a local list", async () => {
it.skip("should allow renaming a local list", async () => {
// Initial set up
const dbConfig = createDbConfig({
localLists: [
@@ -413,7 +394,7 @@ describe("db config store", () => {
configStore.dispose();
});
it("should allow renaming of a local db", async () => {
it.skip("should allow renaming of a local db", async () => {
// Initial set up
const dbConfig = createDbConfig({
localLists: [
@@ -723,7 +704,7 @@ describe("db config store", () => {
configStore.dispose();
});
it("should return true if a local db and local list exists", async () => {
it.skip("should return true if a local db and local list exists", async () => {
// Initial set up
const dbConfig = createDbConfig({
localLists: [
@@ -772,8 +753,11 @@ describe("db config store", () => {
configPath: string,
app: App,
): Promise<DbConfigStore> {
if (dbConfig && dbConfig.databases && dbConfig.databases.local) {
delete (dbConfig.databases as any).local;
}
// const config = clearLocalDbs(dbConfig);
await writeJSON(configPath, dbConfig);
const configStore = new DbConfigStore(app, false);
await configStore.initialize();

View File

@@ -31,18 +31,14 @@ describe("db config validation", () => {
const validationOutput = configValidator.validate(dbConfig);
expect(validationOutput).toHaveLength(3);
expect(validationOutput).toHaveLength(2);
expect(validationOutput[0]).toEqual({
kind: DbConfigValidationErrorKind.InvalidConfig,
message: "/databases must have required property 'local'",
});
expect(validationOutput[1]).toEqual({
kind: DbConfigValidationErrorKind.InvalidConfig,
message:
"/databases/variantAnalysis must have required property 'owners'",
});
expect(validationOutput[2]).toEqual({
expect(validationOutput[1]).toEqual({
kind: DbConfigValidationErrorKind.InvalidConfig,
message: "/databases/variantAnalysis must NOT have additional properties",
});

View File

@@ -173,7 +173,7 @@ describe("db manager", () => {
});
});
it("should throw error when adding a new list to a local node", async () => {
it.skip("should add a new local list", async () => {
const dbConfig: DbConfig = createDbConfig({
localLists: [
{
@@ -285,17 +285,9 @@ describe("db manager", () => {
name: "my-list-2",
repositories: ["owner1/repo1", "owner1/repo2"],
});
// Check that the local list has not been renamed
const localLists = dbConfigFileContents.databases.local.lists;
expect(localLists.length).toBe(1);
expect(localLists[0]).toEqual({
name: "my-list-1",
databases: [localDb],
});
});
it("should rename local db list", async () => {
it.skip("should rename local db list", async () => {
const dbConfig = createDbConfig({
remoteLists: [remoteList],
localLists: [localList],
@@ -328,7 +320,7 @@ describe("db manager", () => {
});
});
it("should rename local db outside a list", async () => {
it.skip("should rename local db outside a list", async () => {
const dbConfig = createDbConfig({
localLists: [localList],
localDbs: [localDb],
@@ -357,7 +349,7 @@ describe("db manager", () => {
});
});
it("should rename local db inside a list", async () => {
it.skip("should rename local db inside a list", async () => {
const dbConfig = createDbConfig({
localLists: [localList],
localDbs: [localDb],
@@ -427,10 +419,6 @@ describe("db manager", () => {
repositories: [remoteRepo1, remoteRepo2],
owners: [remoteOwner],
},
local: {
lists: [localList],
databases: [localDb],
},
},
});
});
@@ -452,10 +440,6 @@ describe("db manager", () => {
repositories: [remoteRepo2],
owners: [remoteOwner],
},
local: {
lists: [localList],
databases: [localDb],
},
},
selected: {
kind: SelectedDbItemKind.VariantAnalysisUserDefinedList,
@@ -481,10 +465,6 @@ describe("db manager", () => {
repositories: [remoteRepo1, remoteRepo2],
owners: [],
},
local: {
lists: [localList],
databases: [localDb],
},
},
selected: {
kind: SelectedDbItemKind.VariantAnalysisUserDefinedList,
@@ -493,7 +473,7 @@ describe("db manager", () => {
});
});
it("should remove local db list", async () => {
it.skip("should remove local db list", async () => {
await saveDbConfig(dbConfig);
const localListDbItems = getLocalListDbItems("my-list-1");
@@ -522,7 +502,7 @@ describe("db manager", () => {
});
});
it("should remove local database", async () => {
it.skip("should remove local database", async () => {
await saveDbConfig(dbConfig);
const localDbItems = getLocalDatabaseDbItems("db1");

View File

@@ -35,9 +35,6 @@ describe("Db panel UI commands", () => {
it("should add new remote db list", async () => {
// Add db list
jest.spyOn(window, "showQuickPick").mockResolvedValue({
kind: DbListKind.Remote,
} as AddListQuickPickItem);
jest.spyOn(window, "showInputBox").mockResolvedValue("my-list-1");
await commands.executeCommand(
"codeQLVariantAnalysisRepositories.addNewList",
@@ -55,7 +52,7 @@ describe("Db panel UI commands", () => {
);
});
it("should add new local db list", async () => {
it.skip("should add new local db list", async () => {
// Add db list
jest.spyOn(window, "showQuickPick").mockResolvedValue({
kind: DbListKind.Local,

View File

@@ -48,7 +48,7 @@ describe("db panel rendering nodes", () => {
await remove(workspaceStoragePath);
});
it("should render default local and remote nodes when the config is empty", async () => {
it("should render default remote nodes when the config is empty", async () => {
const dbConfig: DbConfig = createDbConfig();
await saveDbConfig(dbConfig);
@@ -57,7 +57,7 @@ describe("db panel rendering nodes", () => {
expect(dbTreeItems).toBeTruthy();
const items = dbTreeItems!;
expect(items.length).toBe(2);
expect(items.length).toBe(1);
const remoteRootNode = items[0];
expect(remoteRootNode.dbItem).toBeTruthy();
@@ -77,17 +77,6 @@ describe("db panel rendering nodes", () => {
checkRemoteSystemDefinedListItem(systemDefinedListItems[0], 10);
checkRemoteSystemDefinedListItem(systemDefinedListItems[1], 100);
checkRemoteSystemDefinedListItem(systemDefinedListItems[2], 1000);
const localRootNode = items[1];
expect(localRootNode.dbItem).toBeTruthy();
expect(localRootNode.dbItem?.kind).toBe(DbItemKind.RootLocal);
expect(localRootNode.label).toBe("local");
expect(localRootNode.tooltip).toBe("Local databases");
expect(localRootNode.collapsibleState).toBe(
TreeItemCollapsibleState.Collapsed,
);
expect(localRootNode.children).toBeTruthy();
expect(localRootNode.children.length).toBe(0);
});
it("should render remote repository list nodes", async () => {
@@ -199,7 +188,7 @@ describe("db panel rendering nodes", () => {
checkRemoteRepoItem(repoItems[1], "owner1/repo2");
});
it("should render local list nodes", async () => {
it.skip("should render local list nodes", async () => {
const dbConfig: DbConfig = createDbConfig({
localLists: [
{
@@ -284,7 +273,7 @@ describe("db panel rendering nodes", () => {
]);
});
it("should render local database nodes", async () => {
it.skip("should render local database nodes", async () => {
const dbConfig: DbConfig = createDbConfig({
localDbs: [
{