Add version check for db registration

Database registration is available in versions >= 2.4.1

Author: aeisenberg

extensions/ql-vscode/src/cli.ts

@@ -133,7 +133,7 @@ export class CodeQLCliServer implements Disposable {
   nullBuffer: Buffer;
 
   /** Version of current cli, lazily computed by the `getVersion()` method */
-  _version: SemVer | undefined;
+  private _version: SemVer | undefined;
 
   /** Path to current codeQL executable, or undefined if not running yet. */
   codeQlPath: string | undefined;
@@ -699,7 +699,7 @@ export class CodeQLCliServer implements Disposable {
     );
   }
 
-  private async getVersion() {
+  public async getVersion() {
     if (!this._version) {
       this._version = await this.refreshVersion();
     }

extensions/ql-vscode/src/databases.ts

@@ -622,7 +622,7 @@ export class DatabaseManager extends DisposableObject {
             const databaseItem = await this.createDatabaseItemFromPersistedState(progress, token, database);
             try {
               await databaseItem.refresh();
-              await this.registerDatabases(progress, token, databaseItem);
+              await this.registerDatabase(progress, token, databaseItem);
               if (currentDatabaseUri === database.uri) {
                 this.setCurrentDatabaseItem(databaseItem, true);
               }
@@ -697,7 +697,7 @@ export class DatabaseManager extends DisposableObject {
     // Database items reconstituted from persisted state
     // will not have their contents yet.
     if (item.contents?.datasetUri) {
-      await this.registerDatabases(progress, token, item);
+      await this.registerDatabase(progress, token, item);
     }
     // note that we use undefined as the item in order to reset the entire tree
     this._onDidChangeDatabaseItem.fire({
@@ -748,7 +748,7 @@ export class DatabaseManager extends DisposableObject {
     }
 
     // Remove this database item from the allow-list
-    await this.deregisterDatabases(progress, token, item);
+    await this.deregisterDatabase(progress, token, item);
 
     // note that we use undefined as the item in order to reset the entire tree
     this._onDidChangeDatabaseItem.fire({
@@ -757,12 +757,12 @@ export class DatabaseManager extends DisposableObject {
     });
   }
 
-  private async deregisterDatabases(
+  private async deregisterDatabase(
     progress: ProgressCallback,
     token: vscode.CancellationToken,
     dbItem: DatabaseItem,
   ) {
-    if (dbItem.contents) {
+    if (dbItem.contents && (await this.qs.supportsDatabaseRegistration())) {
       const databases: Dataset[] = [{
         dbDir: dbItem.contents.datasetUri.fsPath,
         workingSet: 'default'
@@ -771,12 +771,12 @@ export class DatabaseManager extends DisposableObject {
     }
   }
 
-  private async registerDatabases(
+  private async registerDatabase(
     progress: ProgressCallback,
     token: vscode.CancellationToken,
     dbItem: DatabaseItem,
   ) {
-    if (dbItem.contents) {
+    if (dbItem.contents && (await this.qs.supportsDatabaseRegistration())) {
       const databases: Dataset[] = [{
         dbDir: dbItem.contents.datasetUri.fsPath,
         workingSet: 'default'

extensions/ql-vscode/src/queryserver-client.ts

@@ -8,6 +8,7 @@ import { QueryServerConfig } from './config';
 import { Logger, ProgressReporter } from './logging';
 import { completeQuery, EvaluationResult, progress, ProgressMessage, WithProgressId } from './pure/messages';
 import * as messages from './pure/messages';
+import { SemVer } from 'semver';
 
 type ServerOpts = {
   logger: Logger;
@@ -47,6 +48,12 @@ type WithProgressReporting = (task: (progress: ProgressReporter, token: Cancella
  * to restart it (which disposes the existing process and starts a new one).
  */
 export class QueryServerClient extends DisposableObject {
+
+  /**
+   * Query Server version where database registration was introduced
+   */
+  private static VERSION_WITH_DB_REGISTRATION = new SemVer('2.4.1');
+
   serverProcess?: ServerProcess;
   evaluationResultCallbacks: { [key: number]: (res: EvaluationResult) => void };
   progressCallbacks: { [key: number]: ((res: ProgressMessage) => void) | undefined };
@@ -161,6 +168,10 @@ export class QueryServerClient extends DisposableObject {
     this.evaluationResultCallbacks = {};
   }
 
+  async supportsDatabaseRegistration() {
+    return (await this.cliServer.getVersion()).compare(QueryServerClient.VERSION_WITH_DB_REGISTRATION) >= 0;
+  }
+
   registerCallback(callback: (res: EvaluationResult) => void): number {
     const id = this.nextCallback++;
     this.evaluationResultCallbacks[id] = callback;

extensions/ql-vscode/src/vscode-tests/minimal-workspace/databases.test.ts

@@ -33,6 +33,7 @@ describe('databases', () => {
   let getSpy: sinon.SinonStub;
   let dbChangedHandler: sinon.SinonSpy;
   let sendRequestSpy: sinon.SinonSpy;
+  let supportsDatabaseRegistrationSpy: sinon.SinonStub;
 
   let sandbox: sinon.SinonSandbox;
   let dir: tmp.DirResult;
@@ -48,6 +49,8 @@ describe('databases', () => {
     getSpy.returns([]);
     sendRequestSpy = sandbox.stub();
     dbChangedHandler = sandbox.spy();
+    supportsDatabaseRegistrationSpy = sandbox.stub();
+    supportsDatabaseRegistrationSpy.resolves(true);
     databaseManager = new DatabaseManager(
       {
         workspaceState: {
@@ -59,7 +62,8 @@ describe('databases', () => {
         storagePath: dir.name
       } as unknown as ExtensionContext,
       {
-        sendRequest: sendRequestSpy
+        sendRequest: sendRequestSpy,
+        supportsDatabaseRegistration: supportsDatabaseRegistrationSpy
       } as unknown as QueryServerClient,
       {} as Logger,
     );
@@ -270,6 +274,30 @@ describe('databases', () => {
       // Should have deregistered this database
       expect(sendRequestSpy).to.have.been.calledWith(registerDatabases, registration, {}, {});
     });
+
+    it('should avoid registration when query server does not support it', async () => {
+      // similar test as above, but now pretend query server doesn't support registration
+      supportsDatabaseRegistrationSpy.resolves(false);
+      const mockDbItem = createMockDB();
+      sandbox.stub(fs, 'remove').resolves();
+
+      await (databaseManager as any).addDatabaseItem(
+        {} as ProgressCallback,
+        {} as CancellationToken,
+        mockDbItem
+      );
+      // Should NOT have registered this database
+      expect(sendRequestSpy).not.to.have.been.called;
+
+      await databaseManager.removeDatabaseItem(
+        {} as ProgressCallback,
+        {} as CancellationToken,
+        mockDbItem
+      );
+
+      // Should NOT have deregistered this database
+      expect(sendRequestSpy).not.to.have.been.called;
+    });
   });
 
   describe('resolveSourceFile', () => {