Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions extensions/ql-vscode/src/pure/helpers-pure.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ export const asyncFilter = async function <T>(arr: T[], predicate: (arg0: T) =>
*/
export const REPO_REGEX = /^[a-zA-Z0-9-_\.]+\/[a-zA-Z0-9-_\.]+$/;

/**
* This regex matches GiHub organization and user strings. These are made up for alphanumeric
* characters, hyphens, underscores or periods.
*/
export const OWNER_REGEX = /^[a-zA-Z0-9-_\.]+$/;

export function getErrorMessage(e: any) {
return e instanceof Error ? e.message : String(e);
}
Expand Down
49 changes: 34 additions & 15 deletions extensions/ql-vscode/src/remote-queries/repository-selection.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
import { QuickPickItem, window } from 'vscode';
import { logger } from '../logging';
import { getRemoteRepositoryLists } from '../config';
import { REPO_REGEX } from '../pure/helpers-pure';
import { OWNER_REGEX, REPO_REGEX } from '../pure/helpers-pure';
import { UserCancellationException } from '../commandRunner';

export interface RepositorySelection {
repositories?: string[];
repositoryLists?: string[]
repositoryLists?: string[];
owners?: string[];
}

interface RepoListQuickPickItem extends QuickPickItem {
repositories?: string[];
repositoryList?: string;
useCustomRepository?: boolean;
useCustomRepo?: boolean;
useAllReposOfOwner?: boolean;
}

/**
Expand All @@ -22,6 +24,7 @@ interface RepoListQuickPickItem extends QuickPickItem {
export async function getRepositorySelection(): Promise<RepositorySelection> {
const quickPickItems = [
createCustomRepoQuickPickItem(),
createAllReposOfOwnerQuickPickItem(),
...createSystemDefinedRepoListsQuickPickItems(),
...createUserDefinedRepoListsQuickPickItems(),
];
Expand All @@ -41,13 +44,20 @@ export async function getRepositorySelection(): Promise<RepositorySelection> {
} else if (quickpick?.repositoryList) {
void logger.log(`Selected repository list: ${quickpick.repositoryList}`);
return { repositoryLists: [quickpick.repositoryList] };
} else if (quickpick?.useCustomRepository) {
} else if (quickpick?.useCustomRepo) {
const customRepo = await getCustomRepo();
if (!customRepo || !REPO_REGEX.test(customRepo)) {
throw new UserCancellationException('Invalid repository format. Please enter a valid repository in the format <owner>/<repo> (e.g. github/codeql)');
}
void logger.log(`Entered repository: ${customRepo}`);
return { repositories: [customRepo] };
} else if (quickpick?.useAllReposOfOwner) {
const owner = await getOwner();
if (!owner || !OWNER_REGEX.test(owner)) {
throw new Error(`Invalid user or organization: ${owner}`);
}
void logger.log(`Entered owner: ${owner}`);
return { owners: [owner] };
} else {
// We don't need to display a warning pop-up in this case, since the user just escaped out of the operation.
// We set 'true' to make this a silent exception.
Expand All @@ -61,17 +71,11 @@ export async function getRepositorySelection(): Promise<RepositorySelection> {
* @returns A boolean flag indicating if the selection is valid or not.
*/
export function isValidSelection(repoSelection: RepositorySelection): boolean {
if (repoSelection.repositories === undefined && repoSelection.repositoryLists === undefined) {
return false;
}
if (repoSelection.repositories !== undefined && repoSelection.repositories.length === 0) {
return false;
}
if (repoSelection.repositoryLists?.length === 0) {
return false;
}
const repositories = repoSelection.repositories || [];
const repositoryLists = repoSelection.repositoryLists || [];
const owners = repoSelection.owners || [];

return true;
return (repositories.length > 0 || repositoryLists.length > 0 || owners.length > 0);
}

function createSystemDefinedRepoListsQuickPickItems(): RepoListQuickPickItem[] {
Expand Down Expand Up @@ -101,11 +105,19 @@ function createUserDefinedRepoListsQuickPickItems(): RepoListQuickPickItem[] {
function createCustomRepoQuickPickItem(): RepoListQuickPickItem {
return {
label: '$(edit) Enter a GitHub repository',
useCustomRepository: true,
useCustomRepo: true,
alwaysShow: true,
};
}

function createAllReposOfOwnerQuickPickItem(): RepoListQuickPickItem {
return {
label: '$(edit) Enter a GitHub user or organization',
useAllReposOfOwner: true,
alwaysShow: true
};
}

async function getCustomRepo(): Promise<string | undefined> {
return await window.showInputBox({
title: 'Enter a GitHub repository in the format <owner>/<repo> (e.g. github/codeql)',
Expand All @@ -114,3 +126,10 @@ async function getCustomRepo(): Promise<string | undefined> {
ignoreFocusOut: true,
});
}

async function getOwner(): Promise<string | undefined> {
return await window.showInputBox({
title: 'Enter a GitHub user or organization',
ignoreFocusOut: true,
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,7 @@ async function runRemoteQueriesApiRequest(
language,
repositories: repoSelection.repositories ?? undefined,
repository_lists: repoSelection.repositoryLists ?? undefined,
repository_owners: repoSelection.owners ?? undefined,
query_pack: queryPackBase64,
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ describe('repository-selection', function() {
});

it('should allow selection from repo lists from your pre-defined config', async () => {
// fake return values
// Fake return values
quickPickSpy.resolves(
{ repositories: ['foo/bar', 'foo/baz'] }
);
Expand All @@ -42,18 +42,19 @@ describe('repository-selection', function() {
}
);

// make the function call
// Make the function call
const repoSelection = await mod.getRepositorySelection();

// Check that the return value is correct
expect(repoSelection.repositoryLists).to.be.undefined;
expect(repoSelection.owners).to.be.undefined;
expect(repoSelection.repositories).to.deep.eq(
['foo/bar', 'foo/baz']
);
});

it('should allow selection from repo lists defined at the system level', async () => {
// fake return values
// Fake return values
quickPickSpy.resolves(
{ repositoryList: 'top_100' }
);
Expand All @@ -64,42 +65,91 @@ describe('repository-selection', function() {
}
);

// make the function call
// Make the function call
const repoSelection = await mod.getRepositorySelection();

// Check that the return value is correct
expect(repoSelection.repositories).to.be.undefined;
expect(repoSelection.owners).to.be.undefined;
expect(repoSelection.repositoryLists).to.deep.eq(
['top_100']
);
});

// Test the regex in various "good" cases
// Test the owner regex in various "good" cases
const goodOwners = [
'owner',
'owner-with-hyphens',
'ownerWithNumbers58',
'owner_with_underscores',
'owner.with.periods.'
];
goodOwners.forEach(owner => {
it(`should run on a valid owner that you enter in the text box: ${owner}`, async () => {
// Fake return values
quickPickSpy.resolves(
{ useAllReposOfOwner: true }
);
getRemoteRepositoryListsSpy.returns({}); // no pre-defined repo lists
showInputBoxSpy.resolves(owner);

// Make the function call
const repoSelection = await mod.getRepositorySelection();

// Check that the return value is correct
expect(repoSelection.repositories).to.be.undefined;
expect(repoSelection.repositoryLists).to.be.undefined;
expect(repoSelection.owners).to.deep.eq([owner]);
});
});

// Test the owner regex in various "bad" cases
const badOwners = [
'invalid&owner',
'owner-with-repo/repo'
];
badOwners.forEach(owner => {
it(`should show an error message if you enter an invalid owner in the text box: ${owner}`, async () => {
// Fake return values
quickPickSpy.resolves(
{ useAllReposOfOwner: true }
);
getRemoteRepositoryListsSpy.returns({}); // no pre-defined repo lists
showInputBoxSpy.resolves(owner);

// Function call should throw a UserCancellationException
await expect(mod.getRepositorySelection()).to.be.rejectedWith(Error, `Invalid user or organization: ${owner}`);
});
});

// Test the repo regex in various "good" cases
const goodRepos = [
'owner/repo',
'owner_with.symbols-/repo.with-symbols_',
'ownerWithNumbers58/repoWithNumbers37'
];
goodRepos.forEach(repo => {
it(`should run on a valid repo that you enter in the text box: ${repo}`, async () => {
// fake return values
// Fake return values
quickPickSpy.resolves(
{ useCustomRepository: true }
{ useCustomRepo: true }
);
getRemoteRepositoryListsSpy.returns({}); // no pre-defined repo lists
showInputBoxSpy.resolves(repo);

// make the function call
// Make the function call
const repoSelection = await mod.getRepositorySelection();

// Check that the return value is correct
expect(repoSelection.repositoryLists).to.be.undefined;
expect(repoSelection.owners).to.be.undefined;
expect(repoSelection.repositories).to.deep.equal(
[repo]
);
});
});

// Test the regex in various "bad" cases
// Test the repo regex in various "bad" cases
const badRepos = [
'invalid*owner/repo',
'owner/repo+some&invalid&stuff',
Expand All @@ -108,14 +158,14 @@ describe('repository-selection', function() {
];
badRepos.forEach(repo => {
it(`should show an error message if you enter an invalid repo in the text box: ${repo}`, async () => {
// fake return values
// Fake return values
quickPickSpy.resolves(
{ useCustomRepository: true }
{ useCustomRepo: true }
);
getRemoteRepositoryListsSpy.returns({}); // no pre-defined repo lists
showInputBoxSpy.resolves(repo);

// function call should throw a UserCancellationException
// Function call should throw a UserCancellationException
await expect(mod.getRepositorySelection()).to.be.rejectedWith(UserCancellationException, 'Invalid repository format');
});
});
Expand Down