Merge pull request #1353 from github/aeisenberg/sort-remote-results

Add sorting to variant analysis results
This commit is contained in:
Andrew Eisenberg
2022-05-20 09:23:10 -07:00
committed by GitHub
17 changed files with 364 additions and 47 deletions

View File

@@ -118,6 +118,8 @@ jobs:
- name: Run integration tests (Linux)
if: matrix.os == 'ubuntu-latest'
working-directory: extensions/ql-vscode
env:
VSCODE_CODEQL_GITHUB_TOKEN: '${{ secrets.GITHUB_TOKEN }}'
run: |
sudo apt-get install xvfb
/usr/bin/xvfb-run npm run integration
@@ -125,6 +127,8 @@ jobs:
- name: Run integration tests (Windows)
if: matrix.os == 'windows-latest'
working-directory: extensions/ql-vscode
env:
VSCODE_CODEQL_GITHUB_TOKEN: '${{ secrets.GITHUB_TOKEN }}'
run: |
npm run integration

View File

@@ -35,7 +35,25 @@ export class Credentials {
return c;
}
private async createOctokit(createIfNone: boolean): Promise<Octokit.Octokit | undefined> {
/**
* Initializes an instance of credentials with an octokit instance using
* a token from the user's GitHub account. This method is meant to be
* used non-interactive environments such as tests.
*
* @param overrideToken The GitHub token to use for authentication.
* @returns An instance of credentials.
*/
static async initializeWithToken(overrideToken: string) {
const c = new Credentials();
c.octokit = await c.createOctokit(false, overrideToken);
return c;
}
private async createOctokit(createIfNone: boolean, overrideToken?: string): Promise<Octokit.Octokit | undefined> {
if (overrideToken) {
return new Octokit.Octokit({ auth: overrideToken });
}
const session = await vscode.authentication.getSession(GITHUB_AUTH_PROVIDER_ID, SCOPES, { createIfNone });
if (session) {

View File

@@ -116,7 +116,9 @@ export class AnalysesResultsManager {
const analysisResults: AnalysisResults = {
nwo: analysis.nwo,
status: 'InProgress',
interpretedResults: []
interpretedResults: [],
resultCount: analysis.resultCount,
starCount: analysis.starCount,
};
const queryId = analysis.downloadLink.queryId;
const resultsForQuery = this.internalGetAnalysesResults(queryId);

View File

@@ -1,13 +1,14 @@
import * as unzipper from 'unzipper';
import * as path from 'path';
import * as fs from 'fs-extra';
import { showAndLogWarningMessage, tmpDir } from '../helpers';
import { showAndLogErrorMessage, showAndLogWarningMessage, tmpDir } from '../helpers';
import { Credentials } from '../authentication';
import { logger } from '../logging';
import { RemoteQueryWorkflowResult } from './remote-query-workflow-result';
import { DownloadLink, createDownloadPath } from './download-link';
import { RemoteQuery } from './remote-query';
import { RemoteQueryFailureIndexItem, RemoteQueryResultIndex, RemoteQuerySuccessIndexItem } from './remote-query-result-index';
import { getErrorMessage } from '../pure/helpers-pure';
interface ApiSuccessIndexItem {
nwo: string;
@@ -332,3 +333,71 @@ export async function createGist(
}
return response.data.html_url;
}
const stargazersQuery = `query Stars($repos: String!, $pageSize: Int!, $cursor: String) {
search(
query: $repos
type: REPOSITORY
first: $pageSize
after: $cursor
) {
edges {
node {
... on Repository {
name
owner {
login
}
stargazerCount
}
}
cursor
}
}
}`;
type StargazersQueryResponse = {
search: {
edges: {
cursor: string;
node: {
name: string;
owner: {
login: string;
};
stargazerCount: number;
}
}[]
}
};
export async function getStargazers(credentials: Credentials, nwos: string[], pageSize = 100): Promise<Record<string, number>> {
const octokit = await credentials.getOctokit();
const repos = `repo:${nwos.join(' repo:')} fork:true`;
let cursor = null;
const stargazers: Record<string, number> = {};
try {
do {
const response: StargazersQueryResponse = await octokit.graphql({
query: stargazersQuery,
repos,
pageSize,
cursor
});
cursor = response.search.edges.length === pageSize ? response.search.edges[pageSize - 1].cursor : null;
for (const edge of response.search.edges) {
const node = edge.node;
const owner = node.owner.login;
const name = node.name;
const stargazerCount = node.stargazerCount;
stargazers[`${owner}/${name}`] = stargazerCount;
}
} while (cursor);
} catch (e) {
void showAndLogErrorMessage(`Error retrieving repository metadata for variant analysis: ${getErrorMessage(e)}`);
}
return stargazers;
}

View File

@@ -306,7 +306,8 @@ export class RemoteQueriesInterfaceManager {
databaseSha: analysisResult.databaseSha || 'HEAD',
resultCount: analysisResult.resultCount,
downloadLink: analysisResult.downloadLink,
fileSize: this.formatFileSize(analysisResult.fileSizeInBytes)
fileSize: this.formatFileSize(analysisResult.fileSizeInBytes),
starCount: analysisResult.starCount
}));
}
}

View File

@@ -12,7 +12,7 @@ import { runRemoteQuery } from './run-remote-query';
import { RemoteQueriesInterfaceManager } from './remote-queries-interface';
import { RemoteQuery } from './remote-query';
import { RemoteQueriesMonitor } from './remote-queries-monitor';
import { getRemoteQueryIndex } from './gh-actions-api-client';
import { getRemoteQueryIndex, getStargazers } from './gh-actions-api-client';
import { RemoteQueryResultIndex } from './remote-query-result-index';
import { RemoteQueryResult } from './remote-query-result';
import { DownloadLink } from './download-link';
@@ -181,8 +181,12 @@ export class RemoteQueriesManager extends DisposableObject {
results => this.interfaceManager.setAnalysisResults(results, queryResult.queryId));
}
private mapQueryResult(executionEndTime: number, resultIndex: RemoteQueryResultIndex, queryId: string): RemoteQueryResult {
private mapQueryResult(
executionEndTime: number,
resultIndex: RemoteQueryResultIndex,
queryId: string,
stargazers: Record<string, number>
): RemoteQueryResult {
const analysisSummaries = resultIndex.successes.map(item => ({
nwo: item.nwo,
databaseSha: item.sha || 'HEAD',
@@ -193,6 +197,7 @@ export class RemoteQueriesManager extends DisposableObject {
urlPath: `${resultIndex.artifactsUrlPath}/${item.artifactId}`,
innerFilePath: item.sarifFileSize ? 'results.sarif' : 'results.bqrs',
queryId,
starCount: stargazers[item.nwo]
} as DownloadLink
}));
const analysisFailures = resultIndex.failures.map(item => ({
@@ -279,7 +284,8 @@ export class RemoteQueriesManager extends DisposableObject {
queryItem.completed = true;
queryItem.status = QueryStatus.Completed;
queryItem.failureReason = undefined;
const queryResult = this.mapQueryResult(executionEndTime, resultIndex, queryItem.queryId);
const stargazers = await this.getStargazersCount(resultIndex, credentials);
const queryResult = this.mapQueryResult(executionEndTime, resultIndex, queryItem.queryId, stargazers);
await this.storeJsonFile(queryItem, 'query-result.json', queryResult);
@@ -303,7 +309,12 @@ export class RemoteQueriesManager extends DisposableObject {
}
}
// Pulled from the analysis results manager, so that we can get access to
private async getStargazersCount(resultIndex: RemoteQueryResultIndex, credentials: Credentials) {
const nwos = resultIndex.successes.map(s => s.nwo);
return await getStargazers(credentials, nwos);
}
// Pulled from the analysis results manager, so that we can get access to
// analyses results from the "export results" command.
public getAnalysesResults(queryId: string): AnalysisResults[] {
return [...this.analysesResultsManager.getAnalysesResults(queryId)];

View File

@@ -2,10 +2,10 @@ import { DownloadLink } from './download-link';
import { AnalysisFailure } from './shared/analysis-failure';
export interface RemoteQueryResult {
executionEndTime: number; // Can't use a Date here since it needs to be serialized and desserialized.
analysisSummaries: AnalysisSummary[];
analysisFailures: AnalysisFailure[];
queryId: string;
executionEndTime: number, // Can't use a Date here since it needs to be serialized and desserialized.
analysisSummaries: AnalysisSummary[],
analysisFailures: AnalysisFailure[],
queryId: string,
}
export interface AnalysisSummary {
@@ -13,5 +13,6 @@ export interface AnalysisSummary {
databaseSha: string,
resultCount: number,
downloadLink: DownloadLink,
fileSizeInBytes: number
fileSizeInBytes: number,
starCount?: number,
}

View File

@@ -7,6 +7,8 @@ export interface AnalysisResults {
status: AnalysisResultStatus;
interpretedResults: AnalysisAlert[];
rawResults?: AnalysisRawResults;
resultCount: number,
starCount?: number,
}
export interface AnalysisRawResults {

View File

@@ -2,19 +2,19 @@ import { DownloadLink } from '../download-link';
import { AnalysisFailure } from './analysis-failure';
export interface RemoteQueryResult {
queryTitle: string;
queryFileName: string;
queryFilePath: string;
queryText: string;
language: string;
workflowRunUrl: string;
totalRepositoryCount: number;
affectedRepositoryCount: number;
totalResultCount: number;
executionTimestamp: string;
executionDuration: string;
analysisSummaries: AnalysisSummary[];
analysisFailures: AnalysisFailure[];
queryTitle: string,
queryFileName: string,
queryFilePath: string,
queryText: string,
language: string,
workflowRunUrl: string,
totalRepositoryCount: number,
affectedRepositoryCount: number,
totalResultCount: number,
executionTimestamp: string,
executionDuration: string,
analysisSummaries: AnalysisSummary[],
analysisFailures: AnalysisFailure[],
}
export interface AnalysisSummary {
@@ -23,4 +23,5 @@ export interface AnalysisSummary {
resultCount: number,
downloadLink: DownloadLink,
fileSize: string,
starCount?: number,
}

View File

@@ -21,6 +21,8 @@ import AnalysisAlertResult from './AnalysisAlertResult';
import RawResultsTable from './RawResultsTable';
import RepositoriesSearch from './RepositoriesSearch';
import ActionButton from './ActionButton';
import StarCount from './StarCount';
import SortRepoFilter, { Sort, sorter } from './SortRepoFilter';
const numOfReposInContractedMode = 10;
@@ -125,10 +127,14 @@ const Failures = (queryResult: RemoteQueryResult) => {
const SummaryTitleWithResults = ({
queryResult,
analysesResults
analysesResults,
sort,
setSort
}: {
queryResult: RemoteQueryResult,
analysesResults: AnalysisResults[]
analysesResults: AnalysisResults[],
sort: Sort,
setSort: (sort: Sort) => void
}) => {
const showDownloadButton = queryResult.totalResultCount !== sumAnalysesResults(analysesResults);
@@ -140,6 +146,10 @@ const SummaryTitleWithResults = ({
text="Download all"
onClick={() => downloadAllAnalysesResults(queryResult)} />
}
<SortRepoFilter
sort={sort}
setSort={setSort}
/>
</div>
);
};
@@ -180,7 +190,7 @@ const SummaryItem = ({
analysisSummary: AnalysisSummary,
analysisResults: AnalysisResults | undefined
}) => (
<span>
<>
<span className="vscode-codeql__analysis-item"><RepoIcon size={16} /></span>
<span className="vscode-codeql__analysis-item">{analysisSummary.nwo}</span>
<span className="vscode-codeql__analysis-item"><Badge text={analysisSummary.resultCount.toString()} /></span>
@@ -189,15 +199,20 @@ const SummaryItem = ({
analysisSummary={analysisSummary}
analysisResults={analysisResults} />
</span>
</span>
<StarCount starCount={analysisSummary.starCount} />
</>
);
const Summary = ({
queryResult,
analysesResults
analysesResults,
sort,
setSort
}: {
queryResult: RemoteQueryResult,
analysesResults: AnalysisResults[]
analysesResults: AnalysisResults[],
sort: Sort,
setSort: (sort: Sort) => void
}) => {
const [repoListExpanded, setRepoListExpanded] = useState(false);
const numOfReposToShow = repoListExpanded ? queryResult.analysisSummaries.length : numOfReposInContractedMode;
@@ -209,17 +224,21 @@ const Summary = ({
? <SummaryTitleNoResults />
: <SummaryTitleWithResults
queryResult={queryResult}
analysesResults={analysesResults} />
analysesResults={analysesResults}
sort={sort}
setSort={setSort} />
}
<ul className="vscode-codeql__flat-list">
{queryResult.analysisSummaries.slice(0, numOfReposToShow).map((summary, i) =>
<li key={summary.nwo} className="vscode-codeql__analysis-summaries-list-item">
<SummaryItem
analysisSummary={summary}
analysisResults={analysesResults.find(a => a.nwo === summary.nwo)} />
</li>
)}
{queryResult.analysisSummaries.slice(0, numOfReposToShow)
.sort(sorter(sort))
.map((summary, i) =>
<li key={summary.nwo} className="vscode-codeql__analysis-summaries-list-item">
<SummaryItem
analysisSummary={summary}
analysisResults={analysesResults.find(a => a.nwo === summary.nwo)} />
</li>
)}
</ul>
{
queryResult.analysisSummaries.length > numOfReposInContractedMode &&
@@ -304,11 +323,13 @@ const RepoAnalysisResults = (analysisResults: AnalysisResults) => {
const AnalysesResults = ({
queryResult,
analysesResults,
totalResults
totalResults,
sort,
}: {
queryResult: RemoteQueryResult,
analysesResults: AnalysisResults[],
totalResults: number
totalResults: number,
sort: Sort
}) => {
const totalAnalysesResults = sumAnalysesResults(analysesResults);
const [filterValue, setFilterValue] = React.useState('');
@@ -343,6 +364,7 @@ const AnalysesResults = ({
{analysesResults
.filter(a => a.interpretedResults.length > 0 || a.rawResults)
.filter(a => a.nwo.toLowerCase().includes(filterValue.toLowerCase()))
.sort(sorter(sort))
.map(r =>
<li key={r.nwo} className="vscode-codeql__analyses-results-list-item">
<RepoAnalysisResults {...r} />
@@ -355,6 +377,7 @@ const AnalysesResults = ({
export function RemoteQueries(): JSX.Element {
const [queryResult, setQueryResult] = useState<RemoteQueryResult>(emptyQueryResult);
const [analysesResults, setAnalysesResults] = useState<AnalysisResults[]>([]);
const [sort, setSort] = useState<Sort>('name');
useEffect(() => {
window.addEventListener('message', (evt: MessageEvent) => {
@@ -384,11 +407,16 @@ export function RemoteQueries(): JSX.Element {
<ViewTitle>{queryResult.queryTitle}</ViewTitle>
<QueryInfo {...queryResult} />
<Failures {...queryResult} />
<Summary queryResult={queryResult} analysesResults={analysesResults} />
<Summary
queryResult={queryResult}
analysesResults={analysesResults}
sort={sort}
setSort={setSort} />
<AnalysesResults
queryResult={queryResult}
analysesResults={analysesResults}
totalResults={queryResult.totalResultCount} />
totalResults={queryResult.totalResultCount}
sort={sort} />
</ThemeProvider>
</div>
);

View File

@@ -0,0 +1,78 @@
import * as React from 'react';
import { FilterIcon } from '@primer/octicons-react';
import { ActionList, ActionMenu, IconButton } from '@primer/react';
import styled from 'styled-components';
const SortWrapper = styled.span`
flex-grow: 2;
text-align: right;
margin-right: 0;
`;
export type Sort = 'name' | 'stars' | 'results';
type Props = {
sort: Sort;
setSort: (sort: Sort) => void;
};
type Sortable = {
nwo: string;
starCount?: number;
resultCount?: number;
};
const sortBy = [
{ name: 'Sort by Name', sort: 'name' },
{ name: 'Sort by Results', sort: 'results' },
{ name: 'Sort by Stars', sort: 'stars' },
];
export function sorter(sort: Sort): (left: Sortable, right: Sortable) => number {
// stars and results are highest to lowest
// name is alphabetical
return (left: Sortable, right: Sortable) => {
if (sort === 'stars') {
const stars = (right.starCount || 0) - (left.starCount || 0);
if (stars !== 0) {
return stars;
}
}
if (sort === 'results') {
const results = (right.resultCount || 0) - (left.resultCount || 0);
if (results !== 0) {
return results;
}
}
// Fall back on name compare if results or stars are equal
return left.nwo.localeCompare(right.nwo, undefined, { sensitivity: 'base' });
};
}
// FIXME These styles are not correct. Need to figure out
// why the theme is not being applied to the ActionMenu
const SortRepoFilter = ({ sort, setSort }: Props) => {
return <SortWrapper>
<ActionMenu>
<ActionMenu.Anchor>
<IconButton icon={FilterIcon} variant="invisible" aria-label="Sort results" />
</ActionMenu.Anchor>
<ActionMenu.Overlay width="small" anchorSide="outside-bottom">
<ActionList selectionVariant="single">
{sortBy.map((type, index) => (
<ActionList.Item
key={index}
selected={type.sort === sort} onSelect={() => setSort(type.sort as Sort)}
>
{type.name}
</ActionList.Item>
))}
</ActionList>
</ActionMenu.Overlay>
</ActionMenu>
</SortWrapper>;
};
export default SortRepoFilter;

View File

@@ -0,0 +1,44 @@
import * as React from 'react';
import { StarIcon } from '@primer/octicons-react';
import styled from 'styled-components';
const Star = styled.span`
flex-grow: 2;
text-align: right;
margin-right: 0;
`;
const Count = styled.span`
text-align: left;
width: 2em;
margin-left: 0.5em;
`;
type Props = { starCount?: number };
const StarCount = ({ starCount }: Props) => (
Number.isFinite(starCount) ? (
<>
<Star>
<StarIcon size={16} />
</Star>
<Count>
{displayStars(starCount!)}
</Count>
</>
) : (
<></>
)
);
function displayStars(starCount: number) {
if (starCount > 10000) {
return `${(starCount / 1000).toFixed(0)}k`;
}
if (starCount > 1000) {
return `${(starCount / 1000).toFixed(1)}k`;
}
return starCount.toFixed(0);
}
export default StarCount;

View File

@@ -14,10 +14,12 @@
.vscode-codeql__query-summary-container {
padding-top: 1.5em;
display: flex;
}
.vscode-codeql__analysis-summaries-list-item {
margin-top: 0.5em;
display: flex;
}
.vscode-codeql__analyses-results-list-item {

View File

@@ -4,6 +4,7 @@
{
"nwo": "github/vscode-codeql",
"resultCount": 15,
"starCount": 1,
"fileSizeInBytes": 191025,
"downloadLink": {
"id": "171543249",
@@ -15,6 +16,7 @@
{
"nwo": "other/hucairz",
"resultCount": 15,
"starCount": 1,
"fileSizeInBytes": 191025,
"downloadLink": {
"id": "11111111",
@@ -26,6 +28,7 @@
{
"nwo": "hucairz/i-dont-exist",
"resultCount": 5,
"starCount": 1,
"fileSizeInBytes": 81237,
"downloadLink": {
"id": "999999",

View File

@@ -4,6 +4,7 @@
{
"nwo": "github/vscode-codeql",
"resultCount": 5,
"starCount": 1,
"fileSizeInBytes": 81237,
"downloadLink": {
"id": "171544171",

View File

@@ -1,10 +1,11 @@
import { fail } from 'assert';
import { expect } from 'chai';
import * as sinon from 'sinon';
import { Credentials } from '../../../authentication';
import { cancelRemoteQuery } from '../../../remote-queries/gh-actions-api-client';
import { cancelRemoteQuery, getStargazers as getStargazersCount } from '../../../remote-queries/gh-actions-api-client';
import { RemoteQuery } from '../../../remote-queries/remote-query';
describe('gh-actions-api-client', () => {
describe('gh-actions-api-client mock responses', () => {
let sandbox: sinon.SinonSandbox;
let mockCredentials: Credentials;
let mockResponse: sinon.SinonStub<any, Promise<{ status: number }>>;
@@ -50,3 +51,44 @@ describe('gh-actions-api-client', () => {
}
});
});
describe('gh-actions-api-client real responses', function() {
this.timeout(10000);
it('should get the stargazers for repos', async () => {
if (skip()) {
return;
}
const credentials = await Credentials.initializeWithToken(process.env.VSCODE_CODEQL_GITHUB_TOKEN!);
const stargazers = await getStargazersCount(credentials, [
'github/codeql',
'github/vscode-codeql',
'rails/rails',
'angular/angular',
'github/hucairz' // This one should not be in the list
],
// choose a page size that is small enough to ensure we make multiple requests
2);
const stargazersKeys = Object.keys(stargazers).sort();
expect(stargazersKeys).to.deep.eq([
'angular/angular',
'github/codeql',
'github/vscode-codeql',
'rails/rails',
]);
});
function skip() {
if (!process.env.VSCODE_CODEQL_GITHUB_TOKEN) {
if (process.env.CI) {
fail('The VSCODE_CODEQL_GITHUB_TOKEN must be set to a valid GITHUB token on CI');
} else {
console.log('Skipping gh-actions-api-client real responses tests. To run these tests, set the value VSCODE_CODEQL_GITHUB_TOKEN to a GitHub token.');
}
return true;
}
return false;
}
});

View File

@@ -253,14 +253,20 @@ describe('Remote queries and query history manager', function() {
expect(trimmed[0]).to.deep.eq([{
nwo: 'github/vscode-codeql',
status: 'InProgress',
resultCount: 15,
starCount: 1
}]);
expect(trimmed[1]).to.deep.eq([{
nwo: 'github/vscode-codeql',
status: 'InProgress',
resultCount: 15,
starCount: 1
}, {
nwo: 'other/hucairz',
status: 'InProgress',
resultCount: 15,
starCount: 1
}]);
// there is a third call. It is non-deterministic if
@@ -270,9 +276,13 @@ describe('Remote queries and query history manager', function() {
expect(trimmed[3]).to.deep.eq([{
nwo: 'github/vscode-codeql',
status: 'Completed',
resultCount: 15,
starCount: 1
}, {
nwo: 'other/hucairz',
status: 'Completed',
resultCount: 15,
starCount: 1
}]);
expect(publisher).to.have.callCount(4);