Merge pull request #1717 from github/norascheuch/add-config-validation

Add config validation

Author: norascheuch

extensions/ql-vscode/package-lock.json

@@ -1299,6 +1299,7 @@
     "@primer/react": "^35.0.0",
     "@vscode/codicons": "^0.0.31",
     "@vscode/webview-ui-toolkit": "^1.0.1",
+    "ajv": "^8.11.0",
     "child-process-promise": "^2.2.1",
     "chokidar": "^3.5.3",
     "classnames": "~2.2.6",
@@ -1389,7 +1390,6 @@
     "@typescript-eslint/eslint-plugin": "^4.26.0",
     "@typescript-eslint/parser": "^4.26.0",
     "@vscode/test-electron": "^2.2.0",
-    "ajv": "^8.11.0",
     "ansi-colors": "^4.1.1",
     "applicationinsights": "^2.3.5",
     "babel-loader": "^8.2.5",

extensions/ql-vscode/package.json

@@ -3,20 +3,25 @@ import * as path from 'path';
 import { cloneDbConfig, DbConfig } from './db-config';
 import * as chokidar from 'chokidar';
 import { DisposableObject } from '../pure/disposable-object';
+import { DbConfigValidator } from './db-config-validator';
 
 export class DbConfigStore extends DisposableObject {
   private readonly configPath: string;
+  private readonly configValidator: DbConfigValidator;
 
   private config: DbConfig;
   private configWatcher: chokidar.FSWatcher | undefined;
 
-  public constructor(workspaceStoragePath: string) {
+  public constructor(
+    workspaceStoragePath: string,
+    extensionPath: string) {
     super();
 
     this.configPath = path.join(workspaceStoragePath, 'workspace-databases.json');
 
     this.config = this.createEmptyConfig();
     this.configWatcher = undefined;
+    this.configValidator = new DbConfigValidator(extensionPath);
   }
 
   public async initialize(): Promise<void> {
@@ -37,6 +42,10 @@ export class DbConfigStore extends DisposableObject {
     return this.configPath;
   }
 
+  public validateConfig(): string[] {
+    return this.configValidator.validate(this.config);
+  }
+
   private async loadConfig(): Promise<void> {
     if (!await fs.pathExists(this.configPath)) {
       await fs.writeJSON(this.configPath, this.createEmptyConfig(), { spaces: 2 });

extensions/ql-vscode/src/databases/db-config-store.ts

@@ -0,0 +1,24 @@
+import * as fs from 'fs-extra';
+import * as path from 'path';
+import Ajv from 'ajv';
+import { DbConfig } from './db-config';
+
+export class DbConfigValidator {
+  private readonly schema: any;
+
+  constructor(extensionPath: string) {
+    const schemaPath = path.resolve(extensionPath, 'workspace-databases-schema.json');
+    this.schema = fs.readJsonSync(schemaPath);
+  }
+
+  public validate(dbConfig: DbConfig): string[] {
+    const ajv = new Ajv({ allErrors: true });
+    ajv.validate(this.schema, dbConfig);
+
+    if (ajv.errors) {
+      return ajv.errors.map((error) => `${error.instancePath} ${error.message}`);
+    }
+
+    return [];
+  }
+}

extensions/ql-vscode/src/databases/db-config-validator.ts

@@ -22,7 +22,8 @@ export class DbModule extends DisposableObject {
     void logger.log('Initializing database module');
 
     const storagePath = extensionContext.storageUri?.fsPath || extensionContext.globalStorageUri.fsPath;
-    const dbConfigStore = new DbConfigStore(storagePath);
+    const extensionPath = extensionContext.extensionPath;
+    const dbConfigStore = new DbConfigStore(storagePath, extensionPath);
     await dbConfigStore.initialize();
 
     const dbManager = new DbManager(dbConfigStore);

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

@@ -4,6 +4,7 @@ import { DbConfigStore } from '../../../src/databases/db-config-store';
 import { expect } from 'chai';
 
 describe('db config store', async () => {
+  const extensionPath = path.join(__dirname, '../../..');
   const tempWorkspaceStoragePath = path.join(__dirname, 'test-workspace');
   const testDataStoragePath = path.join(__dirname, 'data');
 
@@ -18,7 +19,7 @@ describe('db config store', async () => {
   it('should create a new config if one does not exist', async () => {
     const configPath = path.join(tempWorkspaceStoragePath, 'workspace-databases.json');
 
-    const configStore = new DbConfigStore(tempWorkspaceStoragePath);
+    const configStore = new DbConfigStore(tempWorkspaceStoragePath, extensionPath);
     await configStore.initialize();
 
     expect(await fs.pathExists(configPath)).to.be.true;
@@ -29,7 +30,7 @@ describe('db config store', async () => {
   });
 
   it('should load an existing config', async () => {
-    const configStore = new DbConfigStore(testDataStoragePath);
+    const configStore = new DbConfigStore(testDataStoragePath, extensionPath);
     await configStore.initialize();
 
     const config = configStore.getConfig();
@@ -44,7 +45,7 @@ describe('db config store', async () => {
   });
 
   it('should not allow modification of the config', async () => {
-    const configStore = new DbConfigStore(testDataStoragePath);
+    const configStore = new DbConfigStore(testDataStoragePath, extensionPath);
     await configStore.initialize();
 
     const config = configStore.getConfig();

extensions/ql-vscode/test/pure-tests/databases/db-config-store.test.ts

@@ -0,0 +1,33 @@
+import { expect } from 'chai';
+import * as path from 'path';
+import { DbConfig } from '../../../src/databases/db-config';
+import { DbConfigValidator } from '../../../src/databases/db-config-validator';
+
+describe('db config validation', async () => {
+  const extensionPath = path.join(__dirname, '../../..');
+  const configValidator = new DbConfigValidator(extensionPath);
+
+  it('should return error when file is not valid', async () => {
+    // We're intentionally bypassing the type check because we'd
+    // like to make sure validation errors are highlighted.
+    const dbConfig = {
+      'remote': {
+        'repositoryLists': [
+          {
+            'name': 'repoList1',
+            'repositories': ['foo/bar', 'foo/baz']
+          }
+        ],
+        'repositories': ['owner/repo1', 'owner/repo2', 'owner/repo3'],
+        'somethingElse': 'bar'
+      }
+    } as any as DbConfig;
+
+    const validationOutput = configValidator.validate(dbConfig);
+
+    expect(validationOutput).to.have.length(2);
+
+    expect(validationOutput[0]).to.deep.equal('/remote must have required property \'owners\'');
+    expect(validationOutput[1]).to.deep.equal('/remote must NOT have additional properties');
+  });
+});