From 377f7965b1d1a7764c9303968176eacdb9944f3f Mon Sep 17 00:00:00 2001 From: Elena Tanasoiu Date: Mon, 18 Jul 2022 16:36:23 +0100 Subject: [PATCH 1/8] Add result count to remote queries in Query History When you run a remote query, we'd like to display more information about it in the Query History panel. At the moment we've improved this [1] by adding the language and number of repositories. In this commit we're also adding the number of results for a remote query. So the final format of the query history item will change from: ` - ` to ` () on x repositories (y results) - ` [1]: https://github.com/github/vscode-codeql/pull/1427 Co-authored-by: Charis Kyriakou Co-authored-by: Shati Patel --- extensions/ql-vscode/package.json | 2 +- extensions/ql-vscode/src/history-item-label-provider.ts | 3 +-- extensions/ql-vscode/src/query-history.ts | 1 + .../ql-vscode/src/remote-queries/remote-queries-manager.ts | 3 +++ .../src/remote-queries/remote-query-history-item.ts | 1 + .../no-workspace/history-item-label-provider.test.ts | 7 ++++--- 6 files changed, 11 insertions(+), 6 deletions(-) diff --git a/extensions/ql-vscode/package.json b/extensions/ql-vscode/package.json index 63957fefe..0d4af67d0 100644 --- a/extensions/ql-vscode/package.json +++ b/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": { diff --git a/extensions/ql-vscode/src/history-item-label-provider.ts b/extensions/ql-vscode/src/history-item-label-provider.ts index f47ec4d55..73fc30dbe 100644 --- a/extensions/ql-vscode/src/history-item-label-provider.ts +++ b/extensions/ql-vscode/src/history-item-label-provider.ts @@ -74,8 +74,7 @@ export class HistoryItemLabelProvider { // Return the number of repositories queried if available. Otherwise, use the controller repository name. d: numRepositoriesQueried ? numRepositoriesLabel : `${item.remoteQuery.controllerRepository.owner}/${item.remoteQuery.controllerRepository.name}`, - // There is no synchronous way to get the results count. - r: '', + r: item.resultCount === undefined ? '' : `(${item.resultCount} results)`, s: item.status, f: path.basename(item.remoteQuery.queryFilePath), '%': '%' diff --git a/extensions/ql-vscode/src/query-history.ts b/extensions/ql-vscode/src/query-history.ts index 924b396f0..971d7bf49 100644 --- a/extensions/ql-vscode/src/query-history.ts +++ b/extensions/ql-vscode/src/query-history.ts @@ -574,6 +574,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; } diff --git a/extensions/ql-vscode/src/remote-queries/remote-queries-manager.ts b/extensions/ql-vscode/src/remote-queries/remote-queries-manager.ts index dec859618..479a80a8b 100644 --- a/extensions/ql-vscode/src/remote-queries/remote-queries-manager.ts +++ b/extensions/ql-vscode/src/remote-queries/remote-queries-manager.ts @@ -42,6 +42,7 @@ export interface UpdatedQueryStatusEvent { status: QueryStatus; failureReason?: string; numRepositoriesQueried?: number; + resultCount?: number; } export class RemoteQueriesManager extends DisposableObject { @@ -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 = queryResult.analysisSummaries.reduce((acc, cur) => acc + cur.resultCount, 0); this.remoteQueryStatusUpdateEventEmitter.fire({ queryId, status: QueryStatus.Completed, numRepositoriesQueried: queryResult.analysisSummaries.length, + resultCount }); await this.storeJsonFile(queryId, 'query-result.json', queryResult); diff --git a/extensions/ql-vscode/src/remote-queries/remote-query-history-item.ts b/extensions/ql-vscode/src/remote-queries/remote-query-history-item.ts index dd83c3f38..f132263e1 100644 --- a/extensions/ql-vscode/src/remote-queries/remote-query-history-item.ts +++ b/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, diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/history-item-label-provider.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/history-item-label-provider.test.ts index f1a378d41..2184a57d6 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/history-item-label-provider.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/history-item-label-provider.test.ts @@ -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', () => { @@ -142,6 +142,7 @@ describe('HistoryItemLabelProvider', () => { numRepositoriesQueried, }, status: 'in progress', + resultCount: 16, } as unknown as RemoteQueryHistoryItem; } }); From 030488a4598c9663c036facd16aab9f30a7ed3d4 Mon Sep 17 00:00:00 2001 From: Elena Tanasoiu Date: Mon, 18 Jul 2022 16:36:48 +0100 Subject: [PATCH 2/8] Make local and remote query results match In the previous commit we're now displaying number of results for remote queries. Previously we could only do this for local queries. Let's make the format match for both types of queries by displaying number of results in parentheses: `(x results)`. Co-authored-by: Shati Patel --- extensions/ql-vscode/src/history-item-label-provider.ts | 2 +- .../no-workspace/history-item-label-provider.test.ts | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/extensions/ql-vscode/src/history-item-label-provider.ts b/extensions/ql-vscode/src/history-item-label-provider.ts index 73fc30dbe..6a7e48906 100644 --- a/extensions/ql-vscode/src/history-item-label-provider.ts +++ b/extensions/ql-vscode/src/history-item-label-provider.ts @@ -57,7 +57,7 @@ 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(), '%': '%', diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/history-item-label-provider.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/history-item-label-provider.test.ts index 2184a57d6..67727edb2 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/history-item-label-provider.test.ts +++ b/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', () => { From 99a784f07261c7a4c8f8a7897c493b7645405202 Mon Sep 17 00:00:00 2001 From: Elena Tanasoiu Date: Mon, 18 Jul 2022 16:39:59 +0100 Subject: [PATCH 3/8] Be able to sort remote queries by number of results Previously we would set all remote query results to -1 when someone attempted to sort queries. We would then only sort local queries as those had access to the number of results. Let's include number of results for remote queries in the sorting. Co-authored-by: Shati Patel --- extensions/ql-vscode/src/query-history.ts | 5 ++--- .../src/vscode-tests/no-workspace/query-history.test.ts | 9 +++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/extensions/ql-vscode/src/query-history.ts b/extensions/ql-vscode/src/query-history.ts index 971d7bf49..e7b223ef4 100644 --- a/extensions/ql-vscode/src/query-history.ts +++ b/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: diff --git 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 index d65631c72..3b8b77c3c 100644 --- 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 @@ -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 }; } From 9c598c2f0665a15429cdc5d2578ce477df869f83 Mon Sep 17 00:00:00 2001 From: Elena Tanasoiu Date: Tue, 19 Jul 2022 10:42:29 +0100 Subject: [PATCH 4/8] Extract pluralize method There are at least 4 different files where this method could DRY things up, so let's extract it. I've chosen to move it to src/helpers.ts but happy to be told there's a better place for shared utility methods like this one. --- extensions/ql-vscode/src/helpers.ts | 8 +++++++ .../src/remote-queries/run-remote-query.ts | 22 +++++++------------ 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/extensions/ql-vscode/src/helpers.ts b/extensions/ql-vscode/src/helpers.ts index 88017e47a..e3af332cd 100644 --- a/extensions/ql-vscode/src/helpers.ts +++ b/extensions/ql-vscode/src/helpers.ts @@ -581,3 +581,11 @@ export async function* walkDirectory(dir: string): AsyncIterableIterator } } } + +/** + * 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}` : ''; +} diff --git a/extensions/ql-vscode/src/remote-queries/run-remote-query.ts b/extensions/ql-vscode/src/remote-queries/run-remote-query.ts index e1e25782d..176581b47 100644 --- a/extensions/ql-vscode/src/remote-queries/run-remote-query.ts +++ b/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'; @@ -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 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(numRepositoriesQueried, '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(numRepositoriesQueried, '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 += '.'; From 1b425fc261b6bff5c1c219607197757857a23ecf Mon Sep 17 00:00:00 2001 From: Elena Tanasoiu Date: Tue, 19 Jul 2022 13:25:40 +0100 Subject: [PATCH 5/8] DRY up labels using the new pluralize method --- .../src/history-item-label-provider.ts | 21 ++++++++++++------- .../src/remote-queries/export-results.ts | 11 ++++++---- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/extensions/ql-vscode/src/history-item-label-provider.ts b/extensions/ql-vscode/src/history-item-label-provider.ts index 6a7e48906..2ab583361 100644 --- a/extensions/ql-vscode/src/history-item-label-provider.ts +++ b/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 @@ -64,17 +65,23 @@ export class HistoryItemLabelProvider { }; } - private getRemoteInterpolateReplacements(item: RemoteQueryHistoryItem): InterpolateReplacements { + // Return the number of repositories queried if available. Otherwise, use the controller repository name. + private buildRepoLabel(item: RemoteQueryHistoryItem): string { const numRepositoriesQueried = item.remoteQuery.numRepositoriesQueried; - const numRepositoriesLabel = `${numRepositoriesQueried} ${numRepositoriesQueried === 1 ? 'repository' : 'repositories'}`; + + if (numRepositoriesQueried) { + return pluralize(numRepositoriesQueried, 'repository', 'repositories'); + } + + return `${item.remoteQuery.controllerRepository.owner}/${item.remoteQuery.controllerRepository.name}`; + } + + private getRemoteInterpolateReplacements(item: RemoteQueryHistoryItem): InterpolateReplacements { return { t: new Date(item.remoteQuery.executionStartTime).toLocaleString(env.language), q: `${item.remoteQuery.queryName} (${item.remoteQuery.language})`, - - // Return the number of repositories queried if available. Otherwise, use the controller repository name. - d: numRepositoriesQueried ? numRepositoriesLabel : `${item.remoteQuery.controllerRepository.owner}/${item.remoteQuery.controllerRepository.name}`, - - r: item.resultCount === undefined ? '' : `(${item.resultCount} results)`, + d: this.buildRepoLabel(item), + r: `(${pluralize(item.resultCount, 'result', 'results')})`, s: item.status, f: path.basename(item.remoteQuery.queryFilePath), '%': '%' diff --git a/extensions/ql-vscode/src/remote-queries/export-results.ts b/extensions/ql-vscode/src/remote-queries/export-results.ts index ad6555ea8..2b8f8440c 100644 --- a/extensions/ql-vscode/src/remote-queries/export-results.ts +++ b/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 repositoryCount = query.numRepositoriesQueried ? repositoryLabel : ''; - return `${query.queryName} (${query.language}) ${resultCount} results (${repositoryCount})`; + const resultLabel = pluralize(resultCount, 'result', 'results'); + const repositoryLabel = pluralize(query.numRepositoriesQueried, 'repository', 'repositories'); + return `${query.queryName} (${query.language}) ${resultLabel} (${repositoryLabel})`; }; /** From 26529232f46dd71cf2a037915ca69026771c940b Mon Sep 17 00:00:00 2001 From: Elena Tanasoiu Date: Tue, 19 Jul 2022 10:46:10 +0100 Subject: [PATCH 6/8] Rename numRepositoriesQueries to repositoryCount To make it consistent with `resultCount`. --- .../ql-vscode/src/history-item-label-provider.ts | 6 +++--- .../ql-vscode/src/remote-queries/export-results.ts | 2 +- .../src/remote-queries/remote-queries-manager.ts | 4 ++-- .../ql-vscode/src/remote-queries/remote-query.ts | 2 +- .../src/remote-queries/run-remote-query.ts | 14 +++++++------- .../remote-queries/query-with-results/query.json | 2 +- .../history-item-label-provider.test.ts | 4 ++-- 7 files changed, 17 insertions(+), 17 deletions(-) diff --git a/extensions/ql-vscode/src/history-item-label-provider.ts b/extensions/ql-vscode/src/history-item-label-provider.ts index 2ab583361..df86e51c6 100644 --- a/extensions/ql-vscode/src/history-item-label-provider.ts +++ b/extensions/ql-vscode/src/history-item-label-provider.ts @@ -67,10 +67,10 @@ export class HistoryItemLabelProvider { // Return the number of repositories queried if available. Otherwise, use the controller repository name. private buildRepoLabel(item: RemoteQueryHistoryItem): string { - const numRepositoriesQueried = item.remoteQuery.numRepositoriesQueried; + const repositoryCount = item.remoteQuery.repositoryCount; - if (numRepositoriesQueried) { - return pluralize(numRepositoriesQueried, 'repository', 'repositories'); + if (repositoryCount) { + return pluralize(repositoryCount, 'repository', 'repositories'); } return `${item.remoteQuery.controllerRepository.owner}/${item.remoteQuery.controllerRepository.name}`; diff --git a/extensions/ql-vscode/src/remote-queries/export-results.ts b/extensions/ql-vscode/src/remote-queries/export-results.ts index 2b8f8440c..2031d10a4 100644 --- a/extensions/ql-vscode/src/remote-queries/export-results.ts +++ b/extensions/ql-vscode/src/remote-queries/export-results.ts @@ -110,7 +110,7 @@ export async function exportResultsToGist( const buildGistDescription = (query: RemoteQuery, analysesResults: AnalysisResults[]) => { const resultCount = sumAnalysesResults(analysesResults); const resultLabel = pluralize(resultCount, 'result', 'results'); - const repositoryLabel = pluralize(query.numRepositoriesQueried, 'repository', 'repositories'); + const repositoryLabel = pluralize(query.repositoryCount, 'repository', 'repositories'); return `${query.queryName} (${query.language}) ${resultLabel} (${repositoryLabel})`; }; diff --git a/extensions/ql-vscode/src/remote-queries/remote-queries-manager.ts b/extensions/ql-vscode/src/remote-queries/remote-queries-manager.ts index 479a80a8b..99e96a05f 100644 --- a/extensions/ql-vscode/src/remote-queries/remote-queries-manager.ts +++ b/extensions/ql-vscode/src/remote-queries/remote-queries-manager.ts @@ -41,7 +41,7 @@ export interface UpdatedQueryStatusEvent { queryId: string; status: QueryStatus; failureReason?: string; - numRepositoriesQueried?: number; + repositoryCount?: number; resultCount?: number; } @@ -322,7 +322,7 @@ export class RemoteQueriesManager extends DisposableObject { this.remoteQueryStatusUpdateEventEmitter.fire({ queryId, status: QueryStatus.Completed, - numRepositoriesQueried: queryResult.analysisSummaries.length, + repositoryCount: queryResult.analysisSummaries.length, resultCount }); diff --git a/extensions/ql-vscode/src/remote-queries/remote-query.ts b/extensions/ql-vscode/src/remote-queries/remote-query.ts index 34b13f1c9..db838bec9 100644 --- a/extensions/ql-vscode/src/remote-queries/remote-query.ts +++ b/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; } diff --git a/extensions/ql-vscode/src/remote-queries/run-remote-query.ts b/extensions/ql-vscode/src/remote-queries/run-remote-query.ts index 176581b47..6d2d46b37 100644 --- a/extensions/ql-vscode/src/remote-queries/run-remote-query.ts +++ b/extensions/ql-vscode/src/remote-queries/run-remote-query.ts @@ -271,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, @@ -280,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 }; @@ -356,12 +356,12 @@ const eol2 = os.EOL + os.EOL; // 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 ${pluralize(numRepositoriesQueried, 'repository', 'repositories')}. [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 ${pluralize(numRepositoriesQueried, 'repository', 'repositories')}. 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; @@ -430,7 +430,7 @@ async function buildRemoteQueryEntity( queryStartTime: number, workflowRunId: number, language: string, - numRepositoriesQueried: number + repositoryCount: number ): Promise { // The query name is either the name as specified in the query metadata, or the file name. const queryName = queryMetadata?.name ?? path.basename(queryFilePath); @@ -448,6 +448,6 @@ async function buildRemoteQueryEntity( }, executionStartTime: queryStartTime, actionsWorkflowRunId: workflowRunId, - numRepositoriesQueried: numRepositoriesQueried, + repositoryCount: repositoryCount, }; } diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/data/remote-queries/query-with-results/query.json b/extensions/ql-vscode/src/vscode-tests/no-workspace/data/remote-queries/query-with-results/query.json index 93aafac06..951ed92d7 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/data/remote-queries/query-with-results/query.json +++ b/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 } diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/history-item-label-provider.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/history-item-label-provider.test.ts index 67727edb2..08136bf4a 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/history-item-label-provider.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/history-item-label-provider.test.ts @@ -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,7 +139,7 @@ describe('HistoryItemLabelProvider', () => { name: 'vscode-codeql-integration-tests' }, language: 'javascript', - numRepositoriesQueried, + repositoryCount, }, status: 'in progress', resultCount: 16, From 12a97ecba26a0b763ecdee07ee57320112bced98 Mon Sep 17 00:00:00 2001 From: Elena Tanasoiu Date: Tue, 19 Jul 2022 10:46:47 +0100 Subject: [PATCH 7/8] Shorten param forwarding for repositoryCount --- extensions/ql-vscode/src/remote-queries/run-remote-query.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/ql-vscode/src/remote-queries/run-remote-query.ts b/extensions/ql-vscode/src/remote-queries/run-remote-query.ts index 6d2d46b37..dadef8a9b 100644 --- a/extensions/ql-vscode/src/remote-queries/run-remote-query.ts +++ b/extensions/ql-vscode/src/remote-queries/run-remote-query.ts @@ -448,6 +448,6 @@ async function buildRemoteQueryEntity( }, executionStartTime: queryStartTime, actionsWorkflowRunId: workflowRunId, - repositoryCount: repositoryCount, + repositoryCount, }; } From 13921bf8a20e20e59d196928dea2ecc99971d175 Mon Sep 17 00:00:00 2001 From: Elena Tanasoiu Date: Tue, 19 Jul 2022 12:12:40 +0100 Subject: [PATCH 8/8] Extract sum method for adding up repo results When a queryResult is created, it comes with an array for AnalysisSummaries. There is one summary per repository. We've had to calculate the total number of results for all summaries in multiple places, so let's extract a method for this as well. --- .../src/remote-queries/remote-queries-interface.ts | 14 ++++++++++---- .../src/remote-queries/remote-queries-manager.ts | 6 +++--- .../src/remote-queries/remote-query-result.ts | 7 +++++++ 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/extensions/ql-vscode/src/remote-queries/remote-queries-interface.ts b/extensions/ql-vscode/src/remote-queries/remote-queries-interface.ts index b8ba171e1..efe1613f9 100644 --- a/extensions/ql-vscode/src/remote-queries/remote-queries-interface.ts +++ b/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 { AnalysisSummary as AnalysisResultViewModel } from './shared/remote-query-result'; +import { + 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; diff --git a/extensions/ql-vscode/src/remote-queries/remote-queries-manager.ts b/extensions/ql-vscode/src/remote-queries/remote-queries-manager.ts index 99e96a05f..58f772eef 100644 --- a/extensions/ql-vscode/src/remote-queries/remote-queries-manager.ts +++ b/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'; @@ -250,7 +250,7 @@ export class RemoteQueriesManager extends DisposableObject { } private async askToOpenResults(query: RemoteQuery, queryResult: RemoteQueryResult): Promise { - 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`; @@ -318,7 +318,7 @@ export class RemoteQueriesManager extends DisposableObject { if (resultIndex) { const metadata = await this.getRepositoriesMetadata(resultIndex, credentials); const queryResult = this.mapQueryResult(executionEndTime, resultIndex, queryId, metadata); - const resultCount = queryResult.analysisSummaries.reduce((acc, cur) => acc + cur.resultCount, 0); + const resultCount = sumAnalysisSummariesResults(queryResult.analysisSummaries); this.remoteQueryStatusUpdateEventEmitter.fire({ queryId, status: QueryStatus.Completed, diff --git a/extensions/ql-vscode/src/remote-queries/remote-query-result.ts b/extensions/ql-vscode/src/remote-queries/remote-query-result.ts index 03fa73261..a5cb59cea 100644 --- a/extensions/ql-vscode/src/remote-queries/remote-query-result.ts +++ b/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); +};