Merge pull request #1432 from github/charisk-elena/result-count-on-history-labels

Add result count to remote queries in Query History

extensions/ql-vscode/package.json

@@ -224,7 +224,7 @@
         },
         "codeQL.queryHistory.format": {
           "type": "string",
-          "default": "%q on %d - %s, %r [%t]",
+          "default": "%q on %d - %s %r [%t]",
           "markdownDescription": "Default string for how to label query history items.\n* %t is the time of the query\n* %q is the human-readable query name\n* %f is the query file name\n* %d is the database name\n* %r is the number of results\n* %s is a status string"
         },
         "codeQL.queryHistory.ttl": {

extensions/ql-vscode/src/helpers.ts

@@ -581,3 +581,11 @@ export async function* walkDirectory(dir: string): AsyncIterableIterator<string>
     }
   }
 }
+
+/**
+ * Pluralizes a word.
+ * Example: Returns "N repository" if N is one, "N repositories" otherwise.
+ */
+export function pluralize(numItems: number | undefined, singular: string, plural: string): string {
+  return numItems ? `${numItems} ${numItems === 1 ? singular : plural}` : '';
+}

extensions/ql-vscode/src/history-item-label-provider.ts

@@ -3,6 +3,7 @@ import * as path from 'path';
 import { QueryHistoryConfig } from './config';
 import { LocalQueryInfo, QueryHistoryInfo } from './query-results';
 import { RemoteQueryHistoryItem } from './remote-queries/remote-query-history-item';
+import { pluralize } from './helpers';
 
 interface InterpolateReplacements {
   t: string; // Start time
@@ -57,25 +58,30 @@ export class HistoryItemLabelProvider {
       t: item.startTime,
       q: item.getQueryName(),
       d: item.initialInfo.databaseInfo.name,
-      r: `${resultCount} results`,
+      r: `(${resultCount} results)`,
       s: statusString,
       f: item.getQueryFileName(),
       '%': '%',
     };
   }
 
+  // Return the number of repositories queried if available. Otherwise, use the controller repository name.
+  private buildRepoLabel(item: RemoteQueryHistoryItem): string {
+    const repositoryCount = item.remoteQuery.repositoryCount;
+
+    if (repositoryCount) {
+      return pluralize(repositoryCount, 'repository', 'repositories');
+    }
+
+    return `${item.remoteQuery.controllerRepository.owner}/${item.remoteQuery.controllerRepository.name}`;
+  }
+
   private getRemoteInterpolateReplacements(item: RemoteQueryHistoryItem): InterpolateReplacements {
-    const numRepositoriesQueried = item.remoteQuery.numRepositoriesQueried;
-    const numRepositoriesLabel = `${numRepositoriesQueried} ${numRepositoriesQueried === 1 ? 'repository' : 'repositories'}`;
     return {
       t: new Date(item.remoteQuery.executionStartTime).toLocaleString(env.language),
       q: `${item.remoteQuery.queryName} (${item.remoteQuery.language})`,
-
+      d: this.buildRepoLabel(item),
-      // Return the number of repositories queried if available. Otherwise, use the controller repository name.
+      r: `(${pluralize(item.resultCount, 'result', 'results')})`,
-      d: numRepositoriesQueried ? numRepositoriesLabel : `${item.remoteQuery.controllerRepository.owner}/${item.remoteQuery.controllerRepository.name}`,
-
-      // There is no synchronous way to get the results count.
-      r: '',
       s: item.status,
       f: path.basename(item.remoteQuery.queryFilePath),
       '%': '%'

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

@@ -205,13 +205,12 @@ export class HistoryTreeDataProvider extends DisposableObject {
         ? h2.initialInfo.start.getTime()
         : h2.remoteQuery?.executionStartTime;
 
-      // result count for remote queries is not available here.
       const resultCount1 = h1.t === 'local'
         ? h1.completedQuery?.resultCount ?? -1
-        : -1;
+        : h1.resultCount ?? -1;
       const resultCount2 = h2.t === 'local'
         ? h2.completedQuery?.resultCount ?? -1
-        : -1;
+        : h2.resultCount ?? -1;
 
       switch (this.sortOrder) {
         case SortOrder.NameAsc:
@@ -574,6 +573,7 @@ export class QueryHistoryManager extends DisposableObject {
         const remoteQueryHistoryItem = item as RemoteQueryHistoryItem;
         remoteQueryHistoryItem.status = event.status;
         remoteQueryHistoryItem.failureReason = event.failureReason;
+        remoteQueryHistoryItem.resultCount = event.resultCount;
         if (event.status === QueryStatus.Completed) {
           remoteQueryHistoryItem.completed = true;
         }

extensions/ql-vscode/src/remote-queries/export-results.ts

@@ -4,7 +4,10 @@ import * as fs from 'fs-extra';
 import { window, commands, Uri, ExtensionContext, QuickPickItem, workspace, ViewColumn } from 'vscode';
 import { Credentials } from '../authentication';
 import { UserCancellationException } from '../commandRunner';
-import { showInformationMessageWithAction } from '../helpers';
+import {
+  showInformationMessageWithAction,
+  pluralize
+} from '../helpers';
 import { logger } from '../logging';
 import { QueryHistoryManager } from '../query-history';
 import { createGist } from './gh-actions-api-client';
@@ -106,9 +109,9 @@ export async function exportResultsToGist(
  */
 const buildGistDescription = (query: RemoteQuery, analysesResults: AnalysisResults[]) => {
   const resultCount = sumAnalysesResults(analysesResults);
-  const repositoryLabel = `${query.numRepositoriesQueried} ${query.numRepositoriesQueried === 1 ? 'repository' : 'repositories'}`;
+  const resultLabel = pluralize(resultCount, 'result', 'results');
-  const repositoryCount = query.numRepositoriesQueried ? repositoryLabel : '';
+  const repositoryLabel = pluralize(query.repositoryCount, 'repository', 'repositories');
-  return `${query.queryName} (${query.language}) ${resultCount} results (${repositoryCount})`;
+  return `${query.queryName} (${query.language}) ${resultLabel} (${repositoryLabel})`;
 };
 
 /**

extensions/ql-vscode/src/remote-queries/remote-queries-interface.ts

@@ -18,10 +18,16 @@ import {
 import { Logger } from '../logging';
 import { getHtmlForWebview } from '../interface-utils';
 import { assertNever } from '../pure/helpers-pure';
-import { AnalysisSummary, RemoteQueryResult } from './remote-query-result';
+import {
+  AnalysisSummary,
+  RemoteQueryResult,
+  sumAnalysisSummariesResults
+} from './remote-query-result';
 import { RemoteQuery } from './remote-query';
-import { RemoteQueryResult as RemoteQueryResultViewModel } from './shared/remote-query-result';
+import {
-import { AnalysisSummary as AnalysisResultViewModel } from './shared/remote-query-result';
+  AnalysisSummary as AnalysisResultViewModel,
+  RemoteQueryResult as RemoteQueryResultViewModel
+} from './shared/remote-query-result';
 import { showAndLogWarningMessage } from '../helpers';
 import { URLSearchParams } from 'url';
 import { SHOW_QUERY_TEXT_MSG } from '../query-history';
@@ -73,7 +79,7 @@ export class RemoteQueriesInterfaceManager {
    */
   private buildViewModel(query: RemoteQuery, queryResult: RemoteQueryResult): RemoteQueryResultViewModel {
     const queryFileName = path.basename(query.queryFilePath);
-    const totalResultCount = queryResult.analysisSummaries.reduce((acc, cur) => acc + cur.resultCount, 0);
+    const totalResultCount = sumAnalysisSummariesResults(queryResult.analysisSummaries);
     const executionDuration = this.getDuration(queryResult.executionEndTime, query.executionStartTime);
     const analysisSummaries = this.buildAnalysisSummaries(queryResult.analysisSummaries);
     const totalRepositoryCount = queryResult.analysisSummaries.length;

extensions/ql-vscode/src/remote-queries/remote-queries-manager.ts

@@ -15,7 +15,7 @@ import { RemoteQuery } from './remote-query';
 import { RemoteQueriesMonitor } from './remote-queries-monitor';
 import { getRemoteQueryIndex, getRepositoriesMetadata, RepositoriesMetadata } from './gh-actions-api-client';
 import { RemoteQueryResultIndex } from './remote-query-result-index';
-import { RemoteQueryResult } from './remote-query-result';
+import { RemoteQueryResult, sumAnalysisSummariesResults } from './remote-query-result';
 import { DownloadLink } from './download-link';
 import { AnalysesResultsManager } from './analyses-results-manager';
 import { assertNever } from '../pure/helpers-pure';
@@ -41,7 +41,8 @@ export interface UpdatedQueryStatusEvent {
   queryId: string;
   status: QueryStatus;
   failureReason?: string;
-  numRepositoriesQueried?: number;
+  repositoryCount?: number;
+  resultCount?: number;
 }
 
 export class RemoteQueriesManager extends DisposableObject {
@@ -249,7 +250,7 @@ export class RemoteQueriesManager extends DisposableObject {
   }
 
   private async askToOpenResults(query: RemoteQuery, queryResult: RemoteQueryResult): Promise<void> {
-    const totalResultCount = queryResult.analysisSummaries.reduce((acc, cur) => acc + cur.resultCount, 0);
+    const totalResultCount = sumAnalysisSummariesResults(queryResult.analysisSummaries);
     const totalRepoCount = queryResult.analysisSummaries.length;
     const message = `Query "${query.queryName}" run on ${totalRepoCount} repositories and returned ${totalResultCount} results`;
 
@@ -317,10 +318,12 @@ export class RemoteQueriesManager extends DisposableObject {
     if (resultIndex) {
       const metadata = await this.getRepositoriesMetadata(resultIndex, credentials);
       const queryResult = this.mapQueryResult(executionEndTime, resultIndex, queryId, metadata);
+      const resultCount = sumAnalysisSummariesResults(queryResult.analysisSummaries);
       this.remoteQueryStatusUpdateEventEmitter.fire({
         queryId,
         status: QueryStatus.Completed,
-        numRepositoriesQueried: queryResult.analysisSummaries.length,
+        repositoryCount: queryResult.analysisSummaries.length,
+        resultCount
       });
 
       await this.storeJsonFile(queryId, 'query-result.json', queryResult);

extensions/ql-vscode/src/remote-queries/remote-query-history-item.ts

@@ -7,6 +7,7 @@ import { RemoteQuery } from './remote-query';
 export interface RemoteQueryHistoryItem {
   readonly t: 'remote';
   failureReason?: string;
+  resultCount?: number;
   status: QueryStatus;
   completed: boolean;
   readonly queryId: string,

extensions/ql-vscode/src/remote-queries/remote-query-result.ts

@@ -18,3 +18,10 @@ export interface AnalysisSummary {
   starCount?: number,
   lastUpdated?: number,
 }
+
+/**
+ * Sums up the number of results for all repos queried via a remote query.
+ */
+export const sumAnalysisSummariesResults = (analysisSummaries: AnalysisSummary[]): number => {
+  return analysisSummaries.reduce((acc, cur) => acc + cur.resultCount, 0);
+};

extensions/ql-vscode/src/remote-queries/remote-query.ts

@@ -8,5 +8,5 @@ export interface RemoteQuery {
   controllerRepository: Repository;
   executionStartTime: number; // Use number here since it needs to be serialized and desserialized.
   actionsWorkflowRunId: number;
-  numRepositoriesQueried: number;
+  repositoryCount: number;
 }

extensions/ql-vscode/src/remote-queries/run-remote-query.ts

@@ -11,6 +11,7 @@ import {
   showAndLogErrorMessage,
   showAndLogInformationMessage,
   tryGetQueryMetadata,
+  pluralize,
   tmpDir
 } from '../helpers';
 import { Credentials } from '../authentication';
@@ -270,7 +271,7 @@ export async function runRemoteQuery(
       }
 
       const workflowRunId = apiResponse.workflow_run_id;
-      const numRepositoriesQueried = apiResponse.repositories_queried.length;
+      const repositoryCount = apiResponse.repositories_queried.length;
       const remoteQuery = await buildRemoteQueryEntity(
         queryFile,
         queryMetadata,
@@ -279,7 +280,7 @@ export async function runRemoteQuery(
         queryStartTime,
         workflowRunId,
         language,
-        numRepositoriesQueried);
+        repositoryCount);
 
       // don't return the path because it has been deleted
       return { query: remoteQuery };
@@ -352,44 +353,37 @@ async function runRemoteQueriesApiRequest(
 const eol = os.EOL;
 const eol2 = os.EOL + os.EOL;
 
-/**
- * Returns "N repository" if N is one, "N repositories" otherwise.
- */
-function pluralizeRepositories(numRepositories: number) {
-  return `${numRepositories} ${numRepositories === 1 ? 'repository' : 'repositories'}`;
-}
-
 // exported for testing only
 export function parseResponse(owner: string, repo: string, response: QueriesResponse) {
   const repositoriesQueried = response.repositories_queried;
-  const numRepositoriesQueried = repositoriesQueried.length;
+  const repositoryCount = repositoriesQueried.length;
 
-  const popupMessage = `Successfully scheduled runs on ${pluralizeRepositories(numRepositoriesQueried)}. [Click here to see the progress](https://github.com/${owner}/${repo}/actions/runs/${response.workflow_run_id}).`
+  const popupMessage = `Successfully scheduled runs on ${pluralize(repositoryCount, 'repository', 'repositories')}. [Click here to see the progress](https://github.com/${owner}/${repo}/actions/runs/${response.workflow_run_id}).`
     + (response.errors ? `${eol2}Some repositories could not be scheduled. See extension log for details.` : '');
 
-  let logMessage = `Successfully scheduled runs on ${pluralizeRepositories(numRepositoriesQueried)}. See https://github.com/${owner}/${repo}/actions/runs/${response.workflow_run_id}.`;
+  let logMessage = `Successfully scheduled runs on ${pluralize(repositoryCount, 'repository', 'repositories')}. See https://github.com/${owner}/${repo}/actions/runs/${response.workflow_run_id}.`;
   logMessage += `${eol2}Repositories queried:${eol}${repositoriesQueried.join(', ')}`;
   if (response.errors) {
     const { invalid_repositories, repositories_without_database, private_repositories, cutoff_repositories, cutoff_repositories_count } = response.errors;
     logMessage += `${eol2}Some repositories could not be scheduled.`;
     if (invalid_repositories?.length) {
-      logMessage += `${eol2}${pluralizeRepositories(invalid_repositories.length)} invalid and could not be found:${eol}${invalid_repositories.join(', ')}`;
+      logMessage += `${eol2}${pluralize(invalid_repositories.length, 'repository', 'repositories')} invalid and could not be found:${eol}${invalid_repositories.join(', ')}`;
     }
     if (repositories_without_database?.length) {
-      logMessage += `${eol2}${pluralizeRepositories(repositories_without_database.length)} did not have a CodeQL database available:${eol}${repositories_without_database.join(', ')}`;
+      logMessage += `${eol2}${pluralize(repositories_without_database.length, 'repository', 'repositories')} did not have a CodeQL database available:${eol}${repositories_without_database.join(', ')}`;
       logMessage += `${eol}For each public repository that has not yet been added to the database service, we will try to create a database next time the store is updated.`;
     }
     if (private_repositories?.length) {
-      logMessage += `${eol2}${pluralizeRepositories(private_repositories.length)} not public:${eol}${private_repositories.join(', ')}`;
+      logMessage += `${eol2}${pluralize(private_repositories.length, 'repository', 'repositories')} not public:${eol}${private_repositories.join(', ')}`;
       logMessage += `${eol}When using a public controller repository, only public repositories can be queried.`;
     }
     if (cutoff_repositories_count) {
-      logMessage += `${eol2}${pluralizeRepositories(cutoff_repositories_count)} over the limit for a single request`;
+      logMessage += `${eol2}${pluralize(cutoff_repositories_count, 'repository', 'repositories')} over the limit for a single request`;
       if (cutoff_repositories) {
         logMessage += `:${eol}${cutoff_repositories.join(', ')}`;
         if (cutoff_repositories_count !== cutoff_repositories.length) {
           const moreRepositories = cutoff_repositories_count - cutoff_repositories.length;
-          logMessage += `${eol}...${eol}And another ${pluralizeRepositories(moreRepositories)}.`;
+          logMessage += `${eol}...${eol}And another ${pluralize(moreRepositories, 'repository', 'repositories')}.`;
         }
       } else {
         logMessage += '.';
@@ -436,7 +430,7 @@ async function buildRemoteQueryEntity(
   queryStartTime: number,
   workflowRunId: number,
   language: string,
-  numRepositoriesQueried: number
+  repositoryCount: number
 ): Promise<RemoteQuery> {
   // The query name is either the name as specified in the query metadata, or the file name.
   const queryName = queryMetadata?.name ?? path.basename(queryFilePath);
@@ -454,6 +448,6 @@ async function buildRemoteQueryEntity(
     },
     executionStartTime: queryStartTime,
     actionsWorkflowRunId: workflowRunId,
-    numRepositoriesQueried: numRepositoriesQueried,
+    repositoryCount,
   };
 }

extensions/ql-vscode/src/vscode-tests/no-workspace/data/remote-queries/query-with-results/query.json

@@ -6,5 +6,5 @@
   "controllerRepository": { "owner": "dsp-testing", "name": "qc-controller" },
   "executionStartTime": 1649419081990,
   "actionsWorkflowRunId": 2115000864,
-  "numRepositoriesQueried": 10
+  "repositoryCount": 10
 }

extensions/ql-vscode/src/vscode-tests/no-workspace/history-item-label-provider.test.ts

@@ -27,10 +27,10 @@ describe('HistoryItemLabelProvider', () => {
       expect(labelProvider.getLabel(fqi)).to.eq('xxx');
 
       fqi.userSpecifiedLabel = '%t %q %d %s %f %r %%';
-      expect(labelProvider.getLabel(fqi)).to.eq(`${dateStr} query-name db-name in progress query-file.ql 456 results %`);
+      expect(labelProvider.getLabel(fqi)).to.eq(`${dateStr} query-name db-name in progress query-file.ql (456 results) %`);
 
       fqi.userSpecifiedLabel = '%t %q %d %s %f %r %%::%t %q %d %s %f %r %%';
-      expect(labelProvider.getLabel(fqi)).to.eq(`${dateStr} query-name db-name in progress query-file.ql 456 results %::${dateStr} query-name db-name in progress query-file.ql 456 results %`);
+      expect(labelProvider.getLabel(fqi)).to.eq(`${dateStr} query-name db-name in progress query-file.ql (456 results) %::${dateStr} query-name db-name in progress query-file.ql (456 results) %`);
     });
 
     it('should interpolate query when not user specified', () => {
@@ -40,10 +40,10 @@ describe('HistoryItemLabelProvider', () => {
 
 
       config.format = '%t %q %d %s %f %r %%';
-      expect(labelProvider.getLabel(fqi)).to.eq(`${dateStr} query-name db-name in progress query-file.ql 456 results %`);
+      expect(labelProvider.getLabel(fqi)).to.eq(`${dateStr} query-name db-name in progress query-file.ql (456 results) %`);
 
       config.format = '%t %q %d %s %f %r %%::%t %q %d %s %f %r %%';
-      expect(labelProvider.getLabel(fqi)).to.eq(`${dateStr} query-name db-name in progress query-file.ql 456 results %::${dateStr} query-name db-name in progress query-file.ql 456 results %`);
+      expect(labelProvider.getLabel(fqi)).to.eq(`${dateStr} query-name db-name in progress query-file.ql (456 results) %::${dateStr} query-name db-name in progress query-file.ql (456 results) %`);
     });
 
     it('should get query short label', () => {
@@ -102,17 +102,17 @@ describe('HistoryItemLabelProvider', () => {
 
 
       config.format = '%t %q %d %s %f %r %%';
-      expect(labelProvider.getLabel(fqi)).to.eq(`${dateStr} query-name (javascript) github/vscode-codeql-integration-tests in progress query-file.ql  %`);
+      expect(labelProvider.getLabel(fqi)).to.eq(`${dateStr} query-name (javascript) github/vscode-codeql-integration-tests in progress query-file.ql (16 results) %`);
 
       config.format = '%t %q %d %s %f %r %%::%t %q %d %s %f %r %%';
-      expect(labelProvider.getLabel(fqi)).to.eq(`${dateStr} query-name (javascript) github/vscode-codeql-integration-tests in progress query-file.ql  %::${dateStr} query-name (javascript) github/vscode-codeql-integration-tests in progress query-file.ql  %`);
+      expect(labelProvider.getLabel(fqi)).to.eq(`${dateStr} query-name (javascript) github/vscode-codeql-integration-tests in progress query-file.ql (16 results) %::${dateStr} query-name (javascript) github/vscode-codeql-integration-tests in progress query-file.ql (16 results) %`);
     });
 
     it('should use number of repositories instead of controller repo if available', () => {
       const fqi = createMockRemoteQueryInfo(undefined, 2);
 
       config.format = '%t %q %d %s %f %r %%';
-      expect(labelProvider.getLabel(fqi)).to.eq(`${dateStr} query-name (javascript) 2 repositories in progress query-file.ql  %`);
+      expect(labelProvider.getLabel(fqi)).to.eq(`${dateStr} query-name (javascript) 2 repositories in progress query-file.ql (16 results) %`);
     });
 
     it('should get query short label', () => {
@@ -126,7 +126,7 @@ describe('HistoryItemLabelProvider', () => {
       expect(labelProvider.getShortLabel(fqi)).to.eq('query-name');
     });
 
-    function createMockRemoteQueryInfo(userSpecifiedLabel?: string, numRepositoriesQueried?: number) {
+    function createMockRemoteQueryInfo(userSpecifiedLabel?: string, repositoryCount?: number) {
       return {
         t: 'remote',
         userSpecifiedLabel,
@@ -139,9 +139,10 @@ describe('HistoryItemLabelProvider', () => {
             name: 'vscode-codeql-integration-tests'
           },
           language: 'javascript',
-          numRepositoriesQueried,
+          repositoryCount,
         },
         status: 'in progress',
+        resultCount: 16,
       } as unknown as RemoteQueryHistoryItem;
     }
   });

extensions/ql-vscode/src/vscode-tests/no-workspace/query-history.test.ts

@@ -456,11 +456,11 @@ describe('query-history', () => {
 
   describe('getChildren', () => {
     const history = [
-      item('a', 2, 'remote'),
+      item('a', 2, 'remote', 24),
       item('b', 10, 'local', 20),
       item('c', 5, 'local', 30),
       item('d', 1, 'local', 25),
-      item('e', 6, 'remote'),
+      item('e', 6, 'remote', 5),
     ];
     let treeDataProvider: HistoryTreeDataProvider;
 
@@ -503,7 +503,7 @@ describe('query-history', () => {
     });
 
     it('should get children for result count ascending', async () => {
-      const expected = [history[0], history[4], history[1], history[3], history[2]];
+      const expected = [history[4], history[1], history[0], history[3], history[2]];
       treeDataProvider.sortOrder = SortOrder.CountAsc;
 
       const children = await treeDataProvider.getChildren();
@@ -511,7 +511,7 @@ describe('query-history', () => {
     });
 
     it('should get children for result count descending', async () => {
-      const expected = [history[0], history[4], history[1], history[3], history[2]].reverse();
+      const expected = [history[4], history[1], history[0], history[3], history[2]].reverse();
       treeDataProvider.sortOrder = SortOrder.CountDesc;
 
       const children = await treeDataProvider.getChildren();
@@ -573,6 +573,7 @@ describe('query-history', () => {
             },
             repositories: []
           },
+          resultCount,
           t
         };
       }