Refactor: Invert dependency between query history and remote quries managers (#1396)

Author: charisk

extensions/ql-vscode/src/extension.ts

@@ -95,9 +95,9 @@ import { RemoteQueriesManager } from './remote-queries/remote-queries-manager';
 import { RemoteQueryResult } from './remote-queries/remote-query-result';
 import { URLSearchParams } from 'url';
 import { handleDownloadPacks, handleInstallPackDependencies } from './packaging';
-import { RemoteQueryHistoryItem } from './remote-queries/remote-query-history-item';
 import { HistoryItemLabelProvider } from './history-item-label-provider';
 import { exportRemoteQueryResults } from './remote-queries/export-results';
+import { RemoteQuery } from './remote-queries/remote-query';
 
 /**
  * extension.ts
@@ -451,10 +451,20 @@ async function activateWithInstalledDistribution(
   await fs.ensureDir(queryStorageDir);
   const labelProvider = new HistoryItemLabelProvider(queryHistoryConfigurationListener);
 
+  void logger.log('Initializing results panel interface.');
+  const intm = new InterfaceManager(ctx, dbm, cliServer, queryServerLogger, labelProvider);
+  ctx.subscriptions.push(intm);
+
+  void logger.log('Initializing variant analysis manager.');
+  const rqm = new RemoteQueriesManager(ctx, cliServer, queryStorageDir, logger);
+  ctx.subscriptions.push(rqm);
+
   void logger.log('Initializing query history.');
   const qhm = new QueryHistoryManager(
     qs,
     dbm,
+    intm,
+    rqm,
     queryStorageDir,
     ctx,
     queryHistoryConfigurationListener,
@@ -463,17 +473,11 @@ async function activateWithInstalledDistribution(
       showResultsForComparison(from, to),
   );
 
-  qhm.onWillOpenQueryItem(async item => {
-    if (item.t === 'local' && item.completed) {
-      await showResultsForCompletedQuery(item as CompletedLocalQueryInfo, WebviewReveal.Forced);
-    }
-  });
 
   ctx.subscriptions.push(qhm);
 
-  void logger.log('Initializing results panel interface.');
-  const intm = new InterfaceManager(ctx, dbm, cliServer, queryServerLogger, labelProvider);
-  ctx.subscriptions.push(intm);
+  void logger.log('Reading query history');
+  await qhm.readQueryHistory();
 
   void logger.log('Initializing compare panel interface.');
   const cmpm = new CompareInterfaceManager(
@@ -844,14 +848,6 @@ async function activateWithInstalledDistribution(
     )
   );
 
-  void logger.log('Initializing variant analysis results view.');
-  const rqm = new RemoteQueriesManager(ctx, cliServer, qhm, queryStorageDir, logger);
-  ctx.subscriptions.push(rqm);
-
-  // wait until after the remote queries manager is initialized to read the query history
-  // since the rqm is notified of queries being added.
-  await qhm.readQueryHistory();
-
 
   registerRemoteQueryTextProvider();
 
@@ -884,9 +880,10 @@ async function activateWithInstalledDistribution(
 
   ctx.subscriptions.push(
     commandRunner('codeQL.monitorRemoteQuery', async (
-      queryItem: RemoteQueryHistoryItem,
+      queryId: string,
+      query: RemoteQuery,
       token: CancellationToken) => {
-      await rqm.monitorRemoteQuery(queryItem, token);
+      await rqm.monitorRemoteQuery(queryId, query, token);
     }));
 
   ctx.subscriptions.push(

extensions/ql-vscode/src/query-history.ts

@@ -40,6 +40,10 @@ import { CliVersionConstraint } from './cli';
 import { HistoryItemLabelProvider } from './history-item-label-provider';
 import { Credentials } from './authentication';
 import { cancelRemoteQuery } from './remote-queries/gh-actions-api-client';
+import { RemoteQueriesManager } from './remote-queries/remote-queries-manager';
+import { RemoteQueryHistoryItem } from './remote-queries/remote-query-history-item';
+import { InterfaceManager } from './interface';
+import { WebviewReveal } from './interface-utils';
 
 /**
  * query-history.ts
@@ -307,21 +311,11 @@ export class QueryHistoryManager extends DisposableObject {
   queryHistoryScrubber: Disposable | undefined;
   private queryMetadataStorageLocation;
 
-  private readonly _onDidAddQueryItem = super.push(new EventEmitter<QueryHistoryInfo>());
-  readonly onDidAddQueryItem: Event<QueryHistoryInfo> = this
-    ._onDidAddQueryItem.event;
-
-  private readonly _onDidRemoveQueryItem = super.push(new EventEmitter<QueryHistoryInfo>());
-  readonly onDidRemoveQueryItem: Event<QueryHistoryInfo> = this
-    ._onDidRemoveQueryItem.event;
-
-  private readonly _onWillOpenQueryItem = super.push(new EventEmitter<QueryHistoryInfo>());
-  readonly onWillOpenQueryItem: Event<QueryHistoryInfo> = this
-    ._onWillOpenQueryItem.event;
-
   constructor(
     private readonly qs: QueryServerClient,
     private readonly dbm: DatabaseManager,
+    private readonly localQueriesInterfaceManager: InterfaceManager,
+    private readonly remoteQueriesManager: RemoteQueriesManager,
     private readonly queryStorageDir: string,
     private readonly ctx: ExtensionContext,
     private readonly queryHistoryConfigListener: QueryHistoryConfig,
@@ -525,6 +519,7 @@ export class QueryHistoryManager extends DisposableObject {
     }));
 
     this.registerQueryHistoryScrubber(queryHistoryConfigListener, ctx);
+    this.registerToRemoteQueriesEvents();
   }
 
   private getCredentials() {
@@ -548,12 +543,49 @@ export class QueryHistoryManager extends DisposableObject {
     );
   }
 
+  private registerToRemoteQueriesEvents() {
+    const queryAddedSubscription = this.remoteQueriesManager.onRemoteQueryAdded(event => {
+      this.addQuery({
+        t: 'remote',
+        status: QueryStatus.InProgress,
+        completed: false,
+        queryId: event.queryId,
+        remoteQuery: event.query,
+      });
+    });
+
+    const queryRemovedSubscription = this.remoteQueriesManager.onRemoteQueryRemoved(async (event) => {
+      const item = this.treeDataProvider.allHistory.find(i => i.t === 'remote' && i.queryId === event.queryId);
+      if (item) {
+        await this.removeRemoteQuery(item as RemoteQueryHistoryItem);
+      }
+    });
+
+    const queryStatusUpdateSubscription = this.remoteQueriesManager.onRemoteQueryStatusUpdate(async (event) => {
+      const item = this.treeDataProvider.allHistory.find(i => i.t === 'remote' && i.queryId === event.queryId);
+      if (item) {
+        const remoteQueryHistoryItem = item as RemoteQueryHistoryItem;
+        remoteQueryHistoryItem.status = event.status;
+        remoteQueryHistoryItem.failureReason = event.failureReason;
+        await this.refreshTreeView();
+      } else {
+        void logger.log('Variant analysis status update event received for unknown variant analysis');
+      }
+    });
+
+    this.push(queryAddedSubscription);
+    this.push(queryRemovedSubscription);
+    this.push(queryStatusUpdateSubscription);
+  }
+
   async readQueryHistory(): Promise<void> {
     void logger.log(`Reading cached query history from '${this.queryMetadataStorageLocation}'.`);
     const history = await slurpQueryHistory(this.queryMetadataStorageLocation);
     this.treeDataProvider.allHistory = history;
-    this.treeDataProvider.allHistory.forEach((item) => {
-      this._onDidAddQueryItem.fire(item);
+    this.treeDataProvider.allHistory.forEach(async (item) => {
+      if (item.t === 'remote') {
+        await this.remoteQueriesManager.rehydrateRemoteQuery(item.queryId, item.remoteQuery, item.status);
+      }
     });
   }
 
@@ -619,26 +651,30 @@ export class QueryHistoryManager extends DisposableObject {
           await item.completedQuery?.query.deleteQuery();
         }
       } else {
-        // Remote queries can be removed locally, but not remotely.
-        // The user must cancel the query on GitHub Actions explicitly.
-        this.treeDataProvider.remove(item);
-        void logger.log(`Deleted ${this.labelProvider.getLabel(item)}.`);
-        if (item.status === QueryStatus.InProgress) {
-          void logger.log('The variant analysis is still running on GitHub Actions. To cancel there, you must go to the workflow run in your browser.');
-        }
-
-        this._onDidRemoveQueryItem.fire(item);
+        await this.removeRemoteQuery(item);
       }
-
     }));
+
     await this.writeQueryHistory();
     const current = this.treeDataProvider.getCurrent();
     if (current !== undefined) {
       await this.treeView.reveal(current, { select: true });
-      this._onWillOpenQueryItem.fire(current);
+      await this.openQueryResults(current);
     }
   }
 
+  private async removeRemoteQuery(item: RemoteQueryHistoryItem): Promise<void> {
+    // Remote queries can be removed locally, but not remotely.
+    // The user must cancel the query on GitHub Actions explicitly.
+    this.treeDataProvider.remove(item);
+    void logger.log(`Deleted ${this.labelProvider.getLabel(item)}.`);
+    if (item.status === QueryStatus.InProgress) {
+      void logger.log('The variant analysis is still running on GitHub Actions. To cancel there, you must go to the workflow run in your browser.');
+    }
+
+    await this.remoteQueriesManager.removeRemoteQuery(item.queryId);
+  }
+
   async handleSortByName() {
     if (this.treeDataProvider.sortOrder === SortOrder.NameAsc) {
       this.treeDataProvider.sortOrder = SortOrder.NameDesc;
@@ -739,7 +775,7 @@ export class QueryHistoryManager extends DisposableObject {
     } else {
       // show results on single click only if query is completed successfully.
       if (finalSingleItem.status === QueryStatus.Completed) {
-        await this._onWillOpenQueryItem.fire(finalSingleItem);
+        await this.openQueryResults(finalSingleItem);
       }
     }
   }
@@ -1027,7 +1063,6 @@ export class QueryHistoryManager extends DisposableObject {
   addQuery(item: QueryHistoryInfo) {
     this.treeDataProvider.pushQuery(item);
     this.updateTreeViewSelectionIfVisible();
-    this._onDidAddQueryItem.fire(item);
   }
 
   /**
@@ -1228,4 +1263,13 @@ the file in the file explorer and dragging it into the workspace.`
     this.treeDataProvider.refresh();
     await this.writeQueryHistory();
   }
+
+  private async openQueryResults(item: QueryHistoryInfo) {
+    if (item.t === 'local') {
+      await this.localQueriesInterfaceManager.showResults(item as CompletedLocalQueryInfo, WebviewReveal.Forced, false);
+    }
+    else if (item.t === 'remote') {
+      await this.remoteQueriesManager.openRemoteQueryResults(item.queryId);
+    }
+  }
 }