Add remote query items to history view

This is another incremental step on the way to saving history. This commit adds remote items to the history view. It adds in progress and completed icons. Users can explicitly remove items. Here is what is _not_ working: 1. Any other query history commands like open results or open query. 2. Seeing items after a restart.
2022-02-15 09:16:56 -08:00
parent 969dd26041
commit 16df990183
7 changed files with 156 additions and 90 deletions
--- a/extensions/ql-vscode/media/globe.svg
+++ b/extensions/ql-vscode/media/globe.svg
@@ -0,0 +1,16 @@
+<svg width="16" height="16" viewBox="0 0 16 16" fill="none" xmlns="http://www.w3.org/2000/svg">
+<circle cx="7.5" cy="7.5" r="7" stroke="#959DA5"/>
+<mask id="mask0_394_2982" style="mask-type:alpha" maskUnits="userSpaceOnUse" x="0" y="0" width="15" height="15">
+<circle cx="7.5" cy="7.5" r="7.5" fill="#C4C4C4"/>
+</mask>
+<g mask="url(#mask0_394_2982)">
+<path d="M14.5 7.5C14.5 9.42971 13.6822 11.1907 12.5493 12.4721C11.4035 13.7683 10.0054 14.5 8.90625 14.5C7.84644 14.5 6.81131 13.8113 6.01569 12.5383C5.22447 11.2724 4.71875 9.49235 4.71875 7.5C4.71875 5.50765 5.22447 3.72765 6.01569 2.4617C6.81131 1.1887 7.84644 0.5 8.90625 0.5C10.0054 0.5 11.4035 1.23172 12.5493 2.52786C13.6822 3.80934 14.5 5.57029 14.5 7.5Z" stroke="#959DA5"/>
+</g>
+<mask id="mask1_394_2982" style="mask-type:alpha" maskUnits="userSpaceOnUse" x="1" y="0" width="16" height="15">
+<circle cx="9.375" cy="7.5" r="7.5" fill="#C4C4C4"/>
+</mask>
+<g mask="url(#mask1_394_2982)">
+<path d="M10.2812 7.5C10.2812 9.49235 9.77553 11.2724 8.98431 12.5383C8.18869 13.8113 7.15356 14.5 6.09375 14.5C4.99456 14.5 3.5965 13.7683 2.45067 12.4721C1.31781 11.1907 0.5 9.42971 0.5 7.5C0.5 5.57029 1.31781 3.80934 2.45067 2.52786C3.5965 1.23172 4.99456 0.5 6.09375 0.5C7.15356 0.5 8.18869 1.1887 8.98431 2.4617C9.77553 3.72765 10.2812 5.50765 10.2812 7.5Z" stroke="#959DA5"/>
+</g>
+<line y1="7.5" x2="15" y2="7.5" stroke="#959DA5"/>
+</svg>
--- a/extensions/ql-vscode/package.json
+++ b/extensions/ql-vscode/package.json
@@ -674,7 +674,7 @@
        {
          "command": "codeQLQueryHistory.removeHistoryItem",
          "group": "9_qlCommands",
-          "when": "viewItem == interpretedResultsItem || viewItem == rawResultsItem || viewItem == cancelledResultsItem"
+          "when": "viewItem == interpretedResultsItem || viewItem == rawResultsItem || viewItem == remoteResultsItem || viewItem == cancelledResultsItem"
        },
        {
          "command": "codeQLQueryHistory.setLabel",
--- a/extensions/ql-vscode/src/extension.ts
+++ b/extensions/ql-vscode/src/extension.ts
@@ -814,7 +814,7 @@ async function activateWithInstalledDistribution(
  );

  void logger.log('Initializing remote queries interface.');
-  const rqm = new RemoteQueriesManager(ctx, cliServer, queryStorageDir, logger);
+  const rqm = new RemoteQueriesManager(ctx, cliServer, qhm, queryStorageDir, logger);

  registerRemoteQueryTextProvider();

--- a/extensions/ql-vscode/src/query-history.ts
+++ b/extensions/ql-vscode/src/query-history.ts
@@ -76,6 +76,11 @@ const FAILED_QUERY_HISTORY_ITEM_ICON = 'media/red-x.svg';
 */
 const LOCAL_SUCCESS_QUERY_HISTORY_ITEM_ICON = 'media/drive.svg';

+/**
+ * Path to icon to display next to a successful remote run.
+ */
+const REMOTE_SUCCESS_QUERY_HISTORY_ITEM_ICON = 'media/globe.svg';
+
 export enum SortOrder {
  NameAsc = 'NameAsc',
  NameDesc = 'NameDesc',
@@ -91,8 +96,6 @@ export enum SortOrder {
 */
 const DOUBLE_CLICK_TIME = 500;

-const NO_QUERY_SELECTED = 'No query selected. Select a query history item you have already run and try again.';
-
 const WORKSPACE_QUERY_HISTORY_FILE = 'workspace-query-history.json';

 /**
@@ -112,6 +115,8 @@ export class HistoryTreeDataProvider extends DisposableObject {

  private localSuccessIconPath: string;

+  private remoteSuccessIconPath: string;
+
  private current: QueryHistoryInfo | undefined;

  constructor(extensionPath: string) {
@@ -124,6 +129,10 @@ export class HistoryTreeDataProvider extends DisposableObject {
      extensionPath,
      LOCAL_SUCCESS_QUERY_HISTORY_ITEM_ICON
    );
+    this.remoteSuccessIconPath = path.join(
+      extensionPath,
+      REMOTE_SUCCESS_QUERY_HISTORY_ITEM_ICON
+    );
  }

  async getTreeItem(element: QueryHistoryInfo): Promise<TreeItem> {
@@ -136,31 +145,32 @@ export class HistoryTreeDataProvider extends DisposableObject {
      tooltip: element.failureReason || element.label
    };

-    if (element.t === 'local') {
-      // Populate the icon and the context value. We use the context value to
-      // control which commands are visible in the context menu.
-      let hasResults;
-      switch (element.status) {
-        case QueryStatus.InProgress:
-          treeItem.iconPath = new ThemeIcon('sync~spin');
-          treeItem.contextValue = 'inProgressResultsItem';
-          break;
-        case QueryStatus.Completed:
+    // Populate the icon and the context value. We use the context value to
+    // control which commands are visible in the context menu.
+    let hasResults;
+    switch (element.status) {
+      case QueryStatus.InProgress:
+        treeItem.iconPath = new ThemeIcon('sync~spin');
+        treeItem.contextValue = 'inProgressResultsItem';
+        break;
+      case QueryStatus.Completed:
+        if (element.t === 'local') {
          hasResults = await element.completedQuery?.query.hasInterpretedResults();
          treeItem.iconPath = this.localSuccessIconPath;
          treeItem.contextValue = hasResults
            ? 'interpretedResultsItem'
            : 'rawResultsItem';
-          break;
-        case QueryStatus.Failed:
-          treeItem.iconPath = this.failedIconPath;
-          treeItem.contextValue = 'cancelledResultsItem';
-          break;
-        default:
-          assertNever(element.status);
-      }
-    } else {
-      // TODO remote queries are not implemented yet.
+        } else {
+          treeItem.iconPath = this.remoteSuccessIconPath;
+          treeItem.contextValue = 'remoteResultsItem';
+        }
+        break;
+      case QueryStatus.Failed:
+        treeItem.iconPath = this.failedIconPath;
+        treeItem.contextValue = 'cancelledResultsItem';
+        break;
+      default:
+        assertNever(element.status);
    }

    return treeItem;
@@ -493,14 +503,10 @@ export class QueryHistoryManager extends DisposableObject {
    multiSelect: QueryHistoryInfo[]
  ): Promise<void> {
    const { finalSingleItem, finalMultiSelect } = this.determineSelection(singleItem, multiSelect);
-    if (!this.assertSingleQuery(finalMultiSelect) || finalSingleItem.t !== 'local') {
+    if (!this.assertSingleQuery(finalMultiSelect) || finalSingleItem?.t !== 'local') {
      return;
    }

-    if (!finalSingleItem) {
-      throw new Error(NO_QUERY_SELECTED);
-    }
-
    const textDocument = await workspace.openTextDocument(
      Uri.file(finalSingleItem.initialInfo.queryPath)
    );
@@ -528,20 +534,30 @@ export class QueryHistoryManager extends DisposableObject {
    const { finalSingleItem, finalMultiSelect } = this.determineSelection(singleItem, multiSelect);
    const toDelete = (finalMultiSelect || [finalSingleItem]);
    await Promise.all(toDelete.map(async (item) => {
-      // TODO Remote queries are not implemented yet
-      if (item.t !== 'local') {
-        return;
-      }
+      if (item.t === 'local') {
+        // Removing in progress local queries is not supported. They must be cancelled first.
+        if (item.status !== QueryStatus.InProgress) {
+          this.treeDataProvider.remove(item);
+          item.completedQuery?.dispose();
+
+          // User has explicitly asked for this query to be removed.
+          // We need to delete it from disk as well.
+          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.

-      // Removing in progress queries is not supported. They must be cancelled first.
-      if (item.status !== QueryStatus.InProgress) {
        this.treeDataProvider.remove(item);
-        item.completedQuery?.dispose();
-
-        // User has explicitly asked for this query to be removed.
-        // We need to delete it from disk as well.
-        await item.completedQuery?.query.deleteQuery();
+        await item.deleteQuery();
+        void logger.log(`Deleted ${item.label}.`);
+        if (item.status === QueryStatus.InProgress) {
+          void showAndLogInformationMessage(
+            'The remote query is still running on GitHub Actions. To cancel there, you must go to the query run in your browser.'
+          );
+        }
      }
+
    }));
    await this.writeQueryHistory();
    const current = this.treeDataProvider.getCurrent();
@@ -581,7 +597,7 @@ export class QueryHistoryManager extends DisposableObject {
  ): Promise<void> {
    const { finalSingleItem, finalMultiSelect } = this.determineSelection(singleItem, multiSelect);

-    if (!this.assertSingleQuery(finalMultiSelect) || finalSingleItem.t !== 'local') {
+    if (!this.assertSingleQuery(finalMultiSelect) || finalSingleItem?.t !== 'local') {
      return;
    }

@@ -605,7 +621,7 @@ export class QueryHistoryManager extends DisposableObject {
    const { finalSingleItem, finalMultiSelect } = this.determineSelection(singleItem, multiSelect);

    try {
-      if (finalSingleItem.t !== 'local') {
+      if (finalSingleItem?.t !== 'local') {
        throw new Error('Please select a local query.');
      }

@@ -629,14 +645,10 @@ export class QueryHistoryManager extends DisposableObject {
    multiSelect: QueryHistoryInfo[]
  ) {
    const { finalSingleItem, finalMultiSelect } = this.determineSelection(singleItem, multiSelect);
-    if (!this.assertSingleQuery(finalMultiSelect) || finalSingleItem.t !== 'local') {
+    if (!this.assertSingleQuery(finalMultiSelect) || finalSingleItem?.t !== 'local') {
      return;
    }

-    if (!finalSingleItem) {
-      throw new Error(NO_QUERY_SELECTED);
-    }
-
    this.treeDataProvider.setCurrentItem(finalSingleItem);

    const now = new Date();
@@ -694,14 +706,10 @@ export class QueryHistoryManager extends DisposableObject {
  ) {
    const { finalSingleItem, finalMultiSelect } = this.determineSelection(singleItem, multiSelect);

-    if (!this.assertSingleQuery(finalMultiSelect) || finalSingleItem.t !== 'local') {
+    if (!this.assertSingleQuery(finalMultiSelect) || finalSingleItem?.t !== 'local') {
      return;
    }

-    if (!finalSingleItem) {
-      throw new Error(NO_QUERY_SELECTED);
-    }
-
    const params = new URLSearchParams({
      isQuickEval: String(!!finalSingleItem.initialInfo.quickEvalPosition),
      queryText: encodeURIComponent(await this.getQueryText(finalSingleItem)),
@@ -719,7 +727,7 @@ export class QueryHistoryManager extends DisposableObject {
  ) {
    const { finalSingleItem, finalMultiSelect } = this.determineSelection(singleItem, multiSelect);

-    if (!this.assertSingleQuery(finalMultiSelect) || finalSingleItem.t !== 'local' || !finalSingleItem.completedQuery) {
+    if (!this.assertSingleQuery(finalMultiSelect) || finalSingleItem?.t !== 'local' || !finalSingleItem.completedQuery) {
      return;
    }

@@ -743,7 +751,7 @@ export class QueryHistoryManager extends DisposableObject {
  ) {
    const { finalSingleItem, finalMultiSelect } = this.determineSelection(singleItem, multiSelect);

-    if (!this.assertSingleQuery(finalMultiSelect) || finalSingleItem.t !== 'local' || !finalSingleItem.completedQuery) {
+    if (!this.assertSingleQuery(finalMultiSelect) || finalSingleItem?.t !== 'local' || !finalSingleItem.completedQuery) {
      return;
    }
    const query = finalSingleItem.completedQuery.query;
@@ -764,7 +772,7 @@ export class QueryHistoryManager extends DisposableObject {
  ) {
    const { finalSingleItem, finalMultiSelect } = this.determineSelection(singleItem, multiSelect);

-    if (!this.assertSingleQuery(finalMultiSelect) || finalSingleItem.t !== 'local' || !finalSingleItem.completedQuery) {
+    if (!this.assertSingleQuery(finalMultiSelect) || finalSingleItem?.t !== 'local' || !finalSingleItem.completedQuery) {
      return;
    }

@@ -779,7 +787,7 @@ export class QueryHistoryManager extends DisposableObject {
  ) {
    const { finalSingleItem, finalMultiSelect } = this.determineSelection(singleItem, multiSelect);

-    if (!this.assertSingleQuery(finalMultiSelect) || finalSingleItem.t !== 'local' || !finalSingleItem.completedQuery) {
+    if (!this.assertSingleQuery(finalMultiSelect) || finalSingleItem?.t !== 'local' || !finalSingleItem.completedQuery) {
      return;
    }

@@ -788,11 +796,12 @@ export class QueryHistoryManager extends DisposableObject {
    );
  }

-  async getQueryText(queryHistoryItem: LocalQueryInfo): Promise<string> {
-    return queryHistoryItem.initialInfo.queryText;
+  async getQueryText(item: QueryHistoryInfo): Promise<string> {
+    // TODO remote queries cannot have their text returned
+    return item.t === 'local' ? item.initialInfo.queryText : '';
  }

-  addQuery(item: LocalQueryInfo) {
+  addQuery(item: QueryHistoryInfo) {
    this.treeDataProvider.pushQuery(item);
    this.updateTreeViewSelectionIfVisible();
  }
@@ -978,7 +987,7 @@ the file in the file explorer and dragging it into the workspace.`
      }
    }

-    // ensure we do not return undefined
+    // ensure we only return undefined if we have neither a single or multi-selecion
    if (singleItem && !multiSelect?.[0]) {
      multiSelect = [singleItem];
    } else if (!singleItem && multiSelect?.[0]) {
--- a/extensions/ql-vscode/src/remote-queries/remote-queries-manager.ts
+++ b/extensions/ql-vscode/src/remote-queries/remote-queries-manager.ts
@@ -18,6 +18,9 @@ import { RemoteQueryResult } from './remote-query-result';
 import { DownloadLink } from './download-link';
 import { AnalysesResultsManager } from './analyses-results-manager';
 import { assertNever } from '../pure/helpers-pure';
+import { RemoteQueryHistoryItem } from './remote-query-history-item';
+import { QueryHistoryManager } from '../query-history';
+import { QueryStatus } from '../query-status';

 const autoDownloadMaxSize = 300 * 1024;
 const autoDownloadMaxCount = 100;
@@ -30,6 +33,7 @@ export class RemoteQueriesManager {
  constructor(
    private readonly ctx: ExtensionContext,
    private readonly cliServer: CodeQLCliServer,
+    private readonly qhm: QueryHistoryManager,
    private readonly storagePath: string,
    logger: Logger,
  ) {
@@ -61,6 +65,13 @@ export class RemoteQueriesManager {
    query: RemoteQuery,
    cancellationToken: CancellationToken
  ): Promise<void> {
+    const queryId = this.createQueryId(query.queryName);
+    await this.prepareStorageDirectory(queryId);
+    await this.storeFile(queryId, 'query.json', query);
+
+    const queryHistoryItem = new RemoteQueryHistoryItem(query.queryName, queryId, this.storagePath);
+    this.qhm.addQuery(queryHistoryItem);
+
    const credentials = await Credentials.initialize(this.ctx);

    const queryWorkflowResult = await this.remoteQueriesMonitor.monitorQuery(query, cancellationToken);
@@ -73,31 +84,30 @@ export class RemoteQueriesManager {
        void showAndLogErrorMessage(`There was an issue retrieving the result for the query ${query.queryName}`);
        return;
      }
-
-      const queryId = this.createQueryId(query.queryName);
-      await this.prepareStorageDirectory(queryId);
+      queryHistoryItem.completed = true;
+      queryHistoryItem.status = QueryStatus.Completed;
      const queryResult = this.mapQueryResult(executionEndTime, resultIndex, queryId);

-      // Write the query result to the storage directory.
-      const queryResultFilePath = path.join(this.storagePath, queryId, 'query-result.json');
-      await fs.writeFile(queryResultFilePath, JSON.stringify(queryResult, null, 2), 'utf8');
+      await this.storeFile(queryId, 'query-result.json', queryResult);

-      // Kick off auto-download of results.
+      // Kick off auto-download of results in the background.
      void commands.executeCommand('codeQL.autoDownloadRemoteQueryResults', queryResult);

-      const totalResultCount = queryResult.analysisSummaries.reduce((acc, cur) => acc + cur.resultCount, 0);
-      const message = `Query "${query.queryName}" run on ${query.repositories.length} repositories and returned ${totalResultCount} results`;
-
-      const shouldOpenView = await showInformationMessageWithAction(message, 'View');
-      if (shouldOpenView) {
-        await this.interfaceManager.showResults(query, queryResult);
-      }
+      // Ask if the user wants to open the results in the background.
+      void this.askToOpenResults(query, queryResult).then(
+        () => { /* do nothing */ },
+        err => {
+          void showAndLogErrorMessage(err);
+        }
+      );
    } else if (queryWorkflowResult.status === 'CompletedUnsuccessfully') {
+      queryHistoryItem.failureReason = queryWorkflowResult.error;
+      queryHistoryItem.status = QueryStatus.Failed;
      await showAndLogErrorMessage(`Remote query execution failed. Error: ${queryWorkflowResult.error}`);
-
    } else if (queryWorkflowResult.status === 'Cancelled') {
+      queryHistoryItem.failureReason = 'Cancelled';
+      queryHistoryItem.status = QueryStatus.Failed;
      await showAndLogErrorMessage('Remote query monitoring was cancelled');
-
    } else if (queryWorkflowResult.status === 'InProgress') {
      // Should not get here
      await showAndLogErrorMessage(`Unexpected status: ${queryWorkflowResult.status}`);
@@ -105,6 +115,7 @@ export class RemoteQueriesManager {
      // Ensure all cases are covered
      assertNever(queryWorkflowResult.status);
    }
+    this.qhm.refreshTreeView();
  }

  public async autoDownloadRemoteQueryResults(
@@ -146,6 +157,16 @@ export class RemoteQueriesManager {
    };
  }

+  private async askToOpenResults(query: RemoteQuery, queryResult: RemoteQueryResult): Promise<void> {
+    const totalResultCount = queryResult.analysisSummaries.reduce((acc, cur) => acc + cur.resultCount, 0);
+    const message = `Query "${query.queryName}" run on ${query.repositories.length} repositories and returned ${totalResultCount} results`;
+
+    const shouldOpenView = await showInformationMessageWithAction(message, 'View');
+    if (shouldOpenView) {
+      await this.interfaceManager.showResults(query, queryResult);
+    }
+  }
+
  /**
   * Generates a unique id for this query, suitable for determining the storage location for the downloaded query artifacts.
   * @param queryName
@@ -167,4 +188,10 @@ export class RemoteQueriesManager {
  private async prepareStorageDirectory(queryId: string): Promise<void> {
    await createTimestampFile(path.join(this.storagePath, queryId));
  }
+
+  private async storeFile(queryId: string, fileName: string, obj: any): Promise<void> {
+    const filePath = path.join(this.storagePath, queryId, fileName);
+    await fs.writeFile(filePath, JSON.stringify(obj, null, 2), 'utf8');
+  }
+
 }
--- a/extensions/ql-vscode/src/remote-queries/remote-query-history-item.ts
+++ b/extensions/ql-vscode/src/remote-queries/remote-query-history-item.ts
@@ -1,15 +1,33 @@
-
-// TODO This is a stub and will be filled implemented in later PRs.
+import * as fs from 'fs-extra';
+import * as path from 'path';

 import { QueryStatus } from '../query-status';

 /**
 * Information about a remote query.
 */
-export interface RemoteQueryHistoryItem {
-  readonly t: 'remote';
-  label: string;
+export class RemoteQueryHistoryItem {
+  readonly t = 'remote';
  failureReason: string | undefined;
  status: QueryStatus;
-  isCompleted(): boolean;
+  completed = false;
+
+  constructor(
+    public label: string, // TODO, the query label should have interpolation like local queries
+    public readonly queryId: string,
+    private readonly storagePath: string,
+  ) {
+    this.status = QueryStatus.InProgress;
+  }
+
+  isCompleted(): boolean {
+    return this.completed;
+  }
+  async deleteQuery(): Promise<void> {
+    await fs.remove(this.querySaveDir);
+  }
+
+  get querySaveDir(): string {
+    return path.join(this.storagePath, this.queryId);
+  }
 }
--- a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-history.test.ts
+++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-history.test.ts
@@ -207,15 +207,11 @@ describe('query-history', () => {
      expect(queryHistoryManager.treeDataProvider.getCurrent()).to.be.undefined;
    });

-    it('should throw if there is no selection', async () => {
+    it('should do nothing if there is no selection', async () => {
      queryHistoryManager = await createMockQueryHistory(allHistory);
-      try {
-        await queryHistoryManager.handleItemClicked(undefined!, []);
-        expect(true).to.be.false;
-      } catch (e) {
-        expect(selectedCallback).not.to.have.been.called;
-        expect(e.message).to.contain('No query selected');
-      }
+      await queryHistoryManager.handleItemClicked(undefined!, []);
+      expect(selectedCallback).not.to.have.been.called;
+      expect(queryHistoryManager.treeDataProvider.getCurrent()).to.be.undefined;
    });
  });