Merge remote-tracking branch 'origin/main' into koesie10/download-multiple-github-databases

This commit is contained in:
Koen Vlaswinkel
2023-11-20 14:47:18 +01:00
5 changed files with 602 additions and 172 deletions

View File

@@ -0,0 +1,110 @@
import { RequestError } from "@octokit/request-error";
import { Octokit } from "@octokit/rest";
import { RestEndpointMethodTypes } from "@octokit/plugin-rest-endpoint-methods";
import { showNeverAskAgainDialog } from "../common/vscode/dialog";
import { GitHubDatabaseConfig } from "../config";
import { Credentials } from "../common/authentication";
import { AppOctokit } from "../common/octokit";
export type CodeqlDatabase =
RestEndpointMethodTypes["codeScanning"]["listCodeqlDatabases"]["response"]["data"][number];
/**
* Ask the user if they want to connect to GitHub to download CodeQL databases.
* This should be used when the user does not have an access token and should
* be followed by an access token prompt.
*/
async function askForGitHubConnect(
config: GitHubDatabaseConfig,
): Promise<boolean> {
const answer = await showNeverAskAgainDialog(
"This repository has an origin (GitHub) that may have one or more CodeQL databases. Connect to GitHub and download any existing databases?",
false,
"Connect",
"Not now",
"Never",
);
if (answer === "Not now" || answer === undefined) {
return false;
}
if (answer === "Never") {
await config.setDownload("never");
return false;
}
return true;
}
export type ListDatabasesResult = {
/**
* Whether the user has been prompted for credentials. This can be used to determine
* follow-up actions based on whether the user has already had any feedback.
*/
promptedForCredentials: boolean;
databases: CodeqlDatabase[];
octokit: Octokit;
};
/**
* List CodeQL databases for a GitHub repository.
*
* This will first try to fetch the CodeQL databases for the repository with
* existing credentials (or none if there are none). If that fails, it will
* prompt the user to connect to GitHub and try again.
*
* If the user does not want to connect to GitHub, this will return `undefined`.
*/
export async function listDatabases(
owner: string,
repo: string,
credentials: Credentials,
config: GitHubDatabaseConfig,
): Promise<ListDatabasesResult | undefined> {
const hasAccessToken = !!(await credentials.getExistingAccessToken());
let octokit = hasAccessToken
? await credentials.getOctokit()
: new AppOctokit();
let promptedForCredentials = false;
let databases: CodeqlDatabase[];
try {
const response = await octokit.rest.codeScanning.listCodeqlDatabases({
owner,
repo,
});
databases = response.data;
} catch (e) {
// If we get a 404 when we don't have an access token, it might be because
// the repository is private/internal. Therefore, we should ask the user
// whether they want to connect to GitHub and try again.
if (e instanceof RequestError && e.status === 404 && !hasAccessToken) {
// Check whether the user wants to connect to GitHub
if (!(await askForGitHubConnect(config))) {
return;
}
// Prompt for credentials
octokit = await credentials.getOctokit();
promptedForCredentials = true;
const response = await octokit.rest.codeScanning.listCodeqlDatabases({
owner,
repo,
});
databases = response.data;
} else {
throw e;
}
}
return {
promptedForCredentials,
databases,
octokit,
};
}

View File

@@ -1,5 +1,4 @@
import { window } from "vscode";
import { RestEndpointMethodTypes } from "@octokit/plugin-rest-endpoint-methods";
import { Octokit } from "@octokit/rest";
import { showNeverAskAgainDialog } from "../common/vscode/dialog";
import { getLanguageDisplayName } from "../common/query-language";
@@ -9,71 +8,61 @@ import { DatabaseManager } from "./local-databases";
import { CodeQLCliServer } from "../codeql-cli/cli";
import { AppCommandManager } from "../common/commands";
import { GitHubDatabaseConfig } from "../config";
export type CodeqlDatabase =
RestEndpointMethodTypes["codeScanning"]["listCodeqlDatabases"]["response"]["data"][number];
export async function findGitHubDatabasesForRepository(
octokit: Octokit,
owner: string,
repo: string,
): Promise<CodeqlDatabase[]> {
const response = await octokit.rest.codeScanning.listCodeqlDatabases({
owner,
repo,
});
return response.data;
}
import type { CodeqlDatabase } from "./github-database-api";
/**
* Prompt the user to download a database from GitHub. This is a blocking method, so this should
* almost never be called with `await`.
* Ask whether the user wants to download a database from GitHub.
* @return true if the user wants to download a database, false otherwise.
*/
export async function promptGitHubDatabaseDownload(
octokit: Octokit,
owner: string,
repo: string,
export async function askForGitHubDatabaseDownload(
databases: CodeqlDatabase[],
config: GitHubDatabaseConfig,
databaseManager: DatabaseManager,
storagePath: string,
cliServer: CodeQLCliServer,
commandManager: AppCommandManager,
): Promise<void> {
): Promise<boolean> {
const languages = databases.map((database) => database.language);
const databasesMessage =
const message =
databases.length === 1
? `This repository has an origin (GitHub) that has a ${getLanguageDisplayName(
languages[0],
)} CodeQL database.`
)} CodeQL database. Download the existing database from GitHub?`
: `This repository has an origin (GitHub) that has ${joinLanguages(
languages,
)} CodeQL databases.`;
const connectMessage =
databases.length === 1
? `Connect to GitHub and download the existing database?`
: `Connect to GitHub and download any existing databases?`;
)} CodeQL databases. Download any existing databases from GitHub?`;
const answer = await showNeverAskAgainDialog(
`${databasesMessage} ${connectMessage}`,
message,
false,
"Connect",
"Download",
"Not now",
"Never",
);
if (answer === "Not now" || answer === undefined) {
return;
return false;
}
if (answer === "Never") {
await config.setDownload("never");
return;
return false;
}
return true;
}
/**
* Download a database from GitHub by asking the user for a language and then
* downloading the database for that language.
*/
export async function downloadDatabaseFromGitHub(
octokit: Octokit,
owner: string,
repo: string,
databases: CodeqlDatabase[],
databaseManager: DatabaseManager,
storagePath: string,
cliServer: CodeQLCliServer,
commandManager: AppCommandManager,
): Promise<void> {
const selectedDatabases = await promptForDatabases(databases);
if (selectedDatabases.length === 0) {
return;

View File

@@ -1,21 +1,17 @@
import { window } from "vscode";
import { DisposableObject } from "../common/disposable-object";
import { App } from "../common/app";
import { findGitHubRepositoryForWorkspace } from "./github-repository-finder";
import { redactableError } from "../common/errors";
import { asError, getErrorMessage } from "../common/helpers-pure";
import {
CodeqlDatabase,
findGitHubDatabasesForRepository,
promptGitHubDatabaseDownload,
} from "./github-database-prompt";
import {
GitHubDatabaseConfig,
GitHubDatabaseConfigListener,
isCanary,
} from "../config";
import { AppOctokit } from "../common/octokit";
askForGitHubDatabaseDownload,
downloadDatabaseFromGitHub,
} from "./github-database-download";
import { GitHubDatabaseConfig, GitHubDatabaseConfigListener } from "../config";
import { DatabaseManager } from "./local-databases";
import { CodeQLCliServer } from "../codeql-cli/cli";
import { listDatabases, ListDatabasesResult } from "./github-database-api";
export class GithubDatabaseModule extends DisposableObject {
private readonly config: GitHubDatabaseConfig;
@@ -93,18 +89,13 @@ export class GithubDatabaseModule extends DisposableObject {
return;
}
const credentials = isCanary() ? this.app.credentials : undefined;
const octokit = credentials
? await credentials.getOctokit()
: new AppOctokit();
let databases: CodeqlDatabase[];
let result: ListDatabasesResult | undefined;
try {
databases = await findGitHubDatabasesForRepository(
octokit,
result = await listDatabases(
githubRepository.owner,
githubRepository.name,
this.app.credentials,
this.config,
);
} catch (e) {
this.app.telemetry?.sendError(
@@ -120,16 +111,37 @@ export class GithubDatabaseModule extends DisposableObject {
return;
}
if (databases.length === 0) {
// This means the user didn't want to connect, so we can just return.
if (result === undefined) {
return;
}
void promptGitHubDatabaseDownload(
const { databases, promptedForCredentials, octokit } = result;
if (databases.length === 0) {
// If the user didn't have an access token, they have already been prompted,
// so we should give feedback.
if (promptedForCredentials) {
void window.showInformationMessage(
"The GitHub repository does not have any CodeQL databases.",
);
}
return;
}
// If the user already had an access token, first ask if they even want to download the DB.
if (!promptedForCredentials) {
if (!(await askForGitHubDatabaseDownload(databases, this.config))) {
return;
}
}
await downloadDatabaseFromGitHub(
octokit,
githubRepository.owner,
githubRepository.name,
databases,
this.config,
this.databaseManager,
this.databaseStoragePath,
this.cliServer,

View File

@@ -0,0 +1,345 @@
import {
mockedObject,
mockedOctokitFunction,
} from "../../utils/mocking.helpers";
import { GitHubDatabaseConfig } from "../../../../src/config";
import * as dialog from "../../../../src/common/vscode/dialog";
import { listDatabases } from "../../../../src/databases/github-database-api";
import { Credentials } from "../../../../src/common/authentication";
import * as Octokit from "@octokit/rest";
import { AppOctokit } from "../../../../src/common/octokit";
import { RequestError } from "@octokit/request-error";
// Mock the AppOctokit constructor to ensure we aren't making any network requests
jest.mock("../../../../src/common/octokit", () => ({
AppOctokit: jest.fn(),
}));
const appMockListCodeqlDatabases = mockedOctokitFunction<
"codeScanning",
"listCodeqlDatabases"
>();
const appOctokit = mockedObject<Octokit.Octokit>({
rest: {
codeScanning: {
listCodeqlDatabases: appMockListCodeqlDatabases,
},
},
});
beforeEach(() => {
(AppOctokit as unknown as jest.Mock).mockImplementation(() => appOctokit);
});
describe("listDatabases", () => {
const owner = "github";
const repo = "codeql";
const setDownload = jest.fn();
let config: GitHubDatabaseConfig;
let credentials: Credentials;
const mockListCodeqlDatabases = mockedOctokitFunction<
"codeScanning",
"listCodeqlDatabases"
>();
const octokit = mockedObject<Octokit.Octokit>({
rest: {
codeScanning: {
listCodeqlDatabases: mockListCodeqlDatabases,
},
},
});
const databases = [
{
id: 1495869,
name: "csharp-database",
language: "csharp",
uploader: {},
content_type: "application/zip",
state: "uploaded",
size: 55599715,
created_at: "2022-03-24T10:46:24Z",
updated_at: "2022-03-24T10:46:27Z",
url: "https://api.github.com/repositories/143040428/code-scanning/codeql/databases/csharp",
},
];
const successfulMockApiResponse = {
data: databases,
};
let showNeverAskAgainDialogSpy: jest.SpiedFunction<
typeof dialog.showNeverAskAgainDialog
>;
beforeEach(() => {
config = mockedObject<GitHubDatabaseConfig>({
setDownload,
});
mockListCodeqlDatabases.mockResolvedValue(successfulMockApiResponse);
showNeverAskAgainDialogSpy = jest
.spyOn(dialog, "showNeverAskAgainDialog")
.mockResolvedValue("Connect");
});
describe("when the user has an access token", () => {
beforeEach(() => {
credentials = mockedObject<Credentials>({
getExistingAccessToken: () => "ghp_xxx",
getOctokit: () => octokit,
});
});
it("returns the databases", async () => {
expect(await listDatabases(owner, repo, credentials, config)).toEqual({
databases,
promptedForCredentials: false,
octokit,
});
});
describe("when the request fails with a 404", () => {
beforeEach(() => {
mockListCodeqlDatabases.mockRejectedValue(
new RequestError("Not found", 404, {
request: {
method: "GET",
url: "",
headers: {},
},
response: {
status: 404,
headers: {},
url: "",
data: {},
},
}),
);
});
it("throws an error", async () => {
await expect(
listDatabases(owner, repo, credentials, config),
).rejects.toThrowError("Not found");
});
});
describe("when the request fails with a 500", () => {
beforeEach(() => {
mockListCodeqlDatabases.mockRejectedValue(
new RequestError("Internal server error", 500, {
request: {
method: "GET",
url: "",
headers: {},
},
response: {
status: 500,
headers: {},
url: "",
data: {},
},
}),
);
});
it("throws an error", async () => {
await expect(
listDatabases(owner, repo, credentials, config),
).rejects.toThrowError("Internal server error");
});
});
});
describe("when the user does not have an access token", () => {
describe("when the repo is public", () => {
beforeEach(() => {
credentials = mockedObject<Credentials>({
getExistingAccessToken: () => undefined,
});
mockListCodeqlDatabases.mockResolvedValue(undefined);
appMockListCodeqlDatabases.mockResolvedValue(successfulMockApiResponse);
});
it("returns the databases", async () => {
const result = await listDatabases(owner, repo, credentials, config);
expect(result).toEqual({
databases,
promptedForCredentials: false,
octokit: appOctokit,
});
expect(showNeverAskAgainDialogSpy).not.toHaveBeenCalled();
});
describe("when the request fails with a 500", () => {
beforeEach(() => {
appMockListCodeqlDatabases.mockRejectedValue(
new RequestError("Internal server error", 500, {
request: {
method: "GET",
url: "",
headers: {},
},
response: {
status: 500,
headers: {},
url: "",
data: {},
},
}),
);
});
it("throws an error", async () => {
await expect(
listDatabases(owner, repo, credentials, config),
).rejects.toThrowError("Internal server error");
expect(mockListCodeqlDatabases).not.toHaveBeenCalled();
});
});
});
describe("when the repo is private", () => {
beforeEach(() => {
credentials = mockedObject<Credentials>({
getExistingAccessToken: () => undefined,
getOctokit: () => octokit,
});
appMockListCodeqlDatabases.mockRejectedValue(
new RequestError("Not found", 404, {
request: {
method: "GET",
url: "",
headers: {},
},
response: {
status: 404,
headers: {},
url: "",
data: {},
},
}),
);
});
describe("when answering connect to prompt", () => {
beforeEach(() => {
showNeverAskAgainDialogSpy.mockResolvedValue("Connect");
});
it("returns the databases", async () => {
const result = await listDatabases(owner, repo, credentials, config);
expect(result).toEqual({
databases,
promptedForCredentials: true,
octokit,
});
expect(showNeverAskAgainDialogSpy).toHaveBeenCalled();
expect(appMockListCodeqlDatabases).toHaveBeenCalled();
expect(mockListCodeqlDatabases).toHaveBeenCalled();
});
describe("when the request fails with a 404", () => {
beforeEach(() => {
mockListCodeqlDatabases.mockRejectedValue(
new RequestError("Not found", 404, {
request: {
method: "GET",
url: "",
headers: {},
},
response: {
status: 404,
headers: {},
url: "",
data: {},
},
}),
);
});
it("throws an error", async () => {
await expect(
listDatabases(owner, repo, credentials, config),
).rejects.toThrowError("Not found");
});
});
describe("when the request fails with a 500", () => {
beforeEach(() => {
mockListCodeqlDatabases.mockRejectedValue(
new RequestError("Internal server error", 500, {
request: {
method: "GET",
url: "",
headers: {},
},
response: {
status: 500,
headers: {},
url: "",
data: {},
},
}),
);
});
it("throws an error", async () => {
await expect(
listDatabases(owner, repo, credentials, config),
).rejects.toThrowError("Internal server error");
});
});
});
describe("when cancelling prompt", () => {
beforeEach(() => {
showNeverAskAgainDialogSpy.mockResolvedValue(undefined);
});
it("returns undefined", async () => {
const result = await listDatabases(owner, repo, credentials, config);
expect(result).toEqual(undefined);
expect(showNeverAskAgainDialogSpy).toHaveBeenCalled();
expect(appMockListCodeqlDatabases).toHaveBeenCalled();
expect(mockListCodeqlDatabases).not.toHaveBeenCalled();
expect(setDownload).not.toHaveBeenCalled();
});
});
describe("when answering not now to prompt", () => {
beforeEach(() => {
showNeverAskAgainDialogSpy.mockResolvedValue("Not now");
});
it("returns undefined", async () => {
const result = await listDatabases(owner, repo, credentials, config);
expect(result).toEqual(undefined);
expect(showNeverAskAgainDialogSpy).toHaveBeenCalled();
expect(appMockListCodeqlDatabases).toHaveBeenCalled();
expect(mockListCodeqlDatabases).not.toHaveBeenCalled();
expect(setDownload).not.toHaveBeenCalled();
});
});
describe("when answering never to prompt", () => {
beforeEach(() => {
showNeverAskAgainDialogSpy.mockResolvedValue("Never");
});
it("returns undefined and sets the config to 'never'", async () => {
const result = await listDatabases(owner, repo, credentials, config);
expect(result).toEqual(undefined);
expect(showNeverAskAgainDialogSpy).toHaveBeenCalled();
expect(appMockListCodeqlDatabases).toHaveBeenCalled();
expect(mockListCodeqlDatabases).not.toHaveBeenCalled();
expect(setDownload).toHaveBeenCalledWith("never");
});
});
});
});
});

View File

@@ -3,23 +3,97 @@ import { Octokit } from "@octokit/rest";
import { QuickPickItem, window } from "vscode";
import { mockedObject, mockedQuickPickItem } from "../../utils/mocking.helpers";
import {
CodeqlDatabase,
promptGitHubDatabaseDownload,
} from "../../../../src/databases/github-database-prompt";
askForGitHubDatabaseDownload,
downloadDatabaseFromGitHub,
} from "../../../../src/databases/github-database-download";
import { DatabaseManager } from "../../../../src/databases/local-databases";
import { GitHubDatabaseConfig } from "../../../../src/config";
import { CodeQLCliServer } from "../../../../src/codeql-cli/cli";
import { createMockCommandManager } from "../../../__mocks__/commandsMock";
import * as databaseFetcher from "../../../../src/databases/database-fetcher";
import * as dialog from "../../../../src/common/vscode/dialog";
import { CodeqlDatabase } from "../../../../src/databases/github-database-api";
describe("promptGitHubDatabaseDownload", () => {
describe("askForGitHubDatabaseDownload", () => {
const setDownload = jest.fn();
let config: GitHubDatabaseConfig;
const databases = [
mockedObject<CodeqlDatabase>({
language: "swift",
url: faker.internet.url({
protocol: "https",
}),
}),
];
let showNeverAskAgainDialogSpy: jest.SpiedFunction<
typeof dialog.showNeverAskAgainDialog
>;
beforeEach(() => {
config = mockedObject<GitHubDatabaseConfig>({
setDownload,
});
showNeverAskAgainDialogSpy = jest
.spyOn(dialog, "showNeverAskAgainDialog")
.mockResolvedValue("Connect");
});
describe("when answering not now to prompt", () => {
beforeEach(() => {
showNeverAskAgainDialogSpy.mockResolvedValue("Not now");
});
it("returns false", async () => {
expect(await askForGitHubDatabaseDownload(databases, config)).toEqual(
false,
);
expect(setDownload).not.toHaveBeenCalled();
});
});
describe("when cancelling prompt", () => {
beforeEach(() => {
showNeverAskAgainDialogSpy.mockResolvedValue(undefined);
});
it("returns false", async () => {
expect(await askForGitHubDatabaseDownload(databases, config)).toEqual(
false,
);
expect(setDownload).not.toHaveBeenCalled();
});
});
describe("when answering never to prompt", () => {
beforeEach(() => {
showNeverAskAgainDialogSpy.mockResolvedValue("Never");
});
it("returns false", async () => {
expect(await askForGitHubDatabaseDownload(databases, config)).toEqual(
false,
);
});
it("sets the config to never", async () => {
await askForGitHubDatabaseDownload(databases, config);
expect(setDownload).toHaveBeenCalledWith("never");
});
});
});
describe("downloadDatabaseFromGitHub", () => {
let octokit: Octokit;
const owner = "github";
const repo = "codeql";
let databaseManager: DatabaseManager;
const setDownload = jest.fn();
let config: GitHubDatabaseConfig;
const storagePath = "/a/b/c/d";
let cliServer: CodeQLCliServer;
const commandManager = createMockCommandManager();
@@ -37,9 +111,6 @@ describe("promptGitHubDatabaseDownload", () => {
}),
];
let showNeverAskAgainDialogSpy: jest.SpiedFunction<
typeof dialog.showNeverAskAgainDialog
>;
let showQuickPickSpy: jest.SpiedFunction<typeof window.showQuickPick>;
let downloadGitHubDatabaseFromUrlSpy: jest.SpiedFunction<
typeof databaseFetcher.downloadGitHubDatabaseFromUrl
@@ -48,14 +119,8 @@ describe("promptGitHubDatabaseDownload", () => {
beforeEach(() => {
octokit = mockedObject<Octokit>({});
databaseManager = mockedObject<DatabaseManager>({});
config = mockedObject<GitHubDatabaseConfig>({
setDownload,
});
cliServer = mockedObject<CodeQLCliServer>({});
showNeverAskAgainDialogSpy = jest
.spyOn(dialog, "showNeverAskAgainDialog")
.mockResolvedValue("Connect");
showQuickPickSpy = jest.spyOn(window, "showQuickPick").mockResolvedValue(
mockedQuickPickItem([
mockedObject<QuickPickItem & { database: CodeqlDatabase }>({
@@ -69,12 +134,11 @@ describe("promptGitHubDatabaseDownload", () => {
});
it("downloads the database", async () => {
await promptGitHubDatabaseDownload(
await downloadDatabaseFromGitHub(
octokit,
owner,
repo,
databases,
config,
databaseManager,
storagePath,
cliServer,
@@ -97,91 +161,6 @@ describe("promptGitHubDatabaseDownload", () => {
true,
false,
);
expect(showQuickPickSpy).not.toHaveBeenCalled();
expect(config.setDownload).not.toHaveBeenCalled();
});
describe("when answering not now to prompt", () => {
beforeEach(() => {
showNeverAskAgainDialogSpy.mockResolvedValue("Not now");
});
it("does not download the database", async () => {
await promptGitHubDatabaseDownload(
octokit,
owner,
repo,
databases,
config,
databaseManager,
storagePath,
cliServer,
commandManager,
);
expect(downloadGitHubDatabaseFromUrlSpy).not.toHaveBeenCalled();
});
});
describe("when cancelling prompt", () => {
beforeEach(() => {
showNeverAskAgainDialogSpy.mockResolvedValue(undefined);
});
it("does not download the database", async () => {
await promptGitHubDatabaseDownload(
octokit,
owner,
repo,
databases,
config,
databaseManager,
storagePath,
cliServer,
commandManager,
);
expect(downloadGitHubDatabaseFromUrlSpy).not.toHaveBeenCalled();
});
});
describe("when answering never to prompt", () => {
beforeEach(() => {
showNeverAskAgainDialogSpy.mockResolvedValue("Never");
});
it("does not download the database", async () => {
await promptGitHubDatabaseDownload(
octokit,
owner,
repo,
databases,
config,
databaseManager,
storagePath,
cliServer,
commandManager,
);
expect(downloadGitHubDatabaseFromUrlSpy).not.toHaveBeenCalled();
});
it('sets the config to "never"', async () => {
await promptGitHubDatabaseDownload(
octokit,
owner,
repo,
databases,
config,
databaseManager,
storagePath,
cliServer,
commandManager,
);
expect(config.setDownload).toHaveBeenCalledTimes(1);
expect(config.setDownload).toHaveBeenCalledWith("never");
});
});
describe("when there are multiple languages", () => {
@@ -219,12 +198,11 @@ describe("promptGitHubDatabaseDownload", () => {
]),
);
await promptGitHubDatabaseDownload(
await downloadDatabaseFromGitHub(
octokit,
owner,
repo,
databases,
config,
databaseManager,
storagePath,
cliServer,
@@ -262,7 +240,6 @@ describe("promptGitHubDatabaseDownload", () => {
],
expect.anything(),
);
expect(config.setDownload).not.toHaveBeenCalled();
});
it("downloads multiple selected languages", async () => {
@@ -277,12 +254,11 @@ describe("promptGitHubDatabaseDownload", () => {
]),
);
await promptGitHubDatabaseDownload(
await downloadDatabaseFromGitHub(
octokit,
owner,
repo,
databases,
config,
databaseManager,
storagePath,
cliServer,
@@ -335,7 +311,6 @@ describe("promptGitHubDatabaseDownload", () => {
],
expect.anything(),
);
expect(config.setDownload).not.toHaveBeenCalled();
});
describe("when not selecting language", () => {
@@ -344,12 +319,11 @@ describe("promptGitHubDatabaseDownload", () => {
});
it("does not download the database", async () => {
await promptGitHubDatabaseDownload(
await downloadDatabaseFromGitHub(
octokit,
owner,
repo,
databases,
config,
databaseManager,
storagePath,
cliServer,