From 18f1a464933e295471d9ce220eb7eab0939ddfbb Mon Sep 17 00:00:00 2001 From: Robert Date: Thu, 7 Mar 2024 11:51:36 +0000 Subject: [PATCH 1/9] Naively convert DatabaseFetcher to a class --- .../src/databases/database-fetcher.ts | 1080 +++++++++-------- .../databases/github-databases/download.ts | 5 +- .../github-databases-module.ts | 6 + .../src/databases/github-databases/updates.ts | 34 +- .../src/databases/local-databases-ui.ts | 14 +- extensions/ql-vscode/src/extension.ts | 2 + .../src/local-queries/local-queries.ts | 2 + .../local-queries/skeleton-query-wizard.ts | 13 +- .../src/model-editor/model-editor-view.ts | 23 +- .../databases/database-fetcher.test.ts | 9 +- .../skeleton-query-wizard.test.ts | 18 +- .../test/vscode-tests/global.helper.ts | 4 +- .../github-databases/download.test.ts | 10 +- .../github-databases-module.test.ts | 7 + .../github-databases/updates.test.ts | 10 +- 15 files changed, 647 insertions(+), 590 deletions(-) diff --git a/extensions/ql-vscode/src/databases/database-fetcher.ts b/extensions/ql-vscode/src/databases/database-fetcher.ts index b40975d3894..4c983e43b0f 100644 --- a/extensions/ql-vscode/src/databases/database-fetcher.ts +++ b/extensions/ql-vscode/src/databases/database-fetcher.ts @@ -42,605 +42,623 @@ import { createFilenameFromString } from "../common/filenames"; import { findDirWithFile } from "../common/files"; import { convertGithubNwoToDatabaseUrl } from "./github-databases/api"; -/** - * Prompts a user to fetch a database from a remote location. Database is assumed to be an archive file. - * - * @param databaseManager the DatabaseManager - * @param storagePath where to store the unzipped database. - */ -export async function promptImportInternetDatabase( - commandManager: AppCommandManager, - databaseManager: DatabaseManager, - storagePath: string, - progress: ProgressCallback, - cli: CodeQLCliServer, -): Promise { - const databaseUrl = await window.showInputBox({ - prompt: "Enter URL of zipfile of database to download", - }); - if (!databaseUrl) { - return; - } +// The number of tries to use when generating a unique filename before +// giving up and using a nanoid. +const DUPLICATE_FILENAMES_TRIES = 10_000; + +export class DatabaseFetcher { + /** + * Prompts a user to fetch a database from a remote location. Database is assumed to be an archive file. + * + * @param databaseManager the DatabaseManager + * @param storagePath where to store the unzipped database. + */ + public async promptImportInternetDatabase( + commandManager: AppCommandManager, + databaseManager: DatabaseManager, + storagePath: string, + progress: ProgressCallback, + cli: CodeQLCliServer, + ): Promise { + const databaseUrl = await window.showInputBox({ + prompt: "Enter URL of zipfile of database to download", + }); + if (!databaseUrl) { + return; + } - validateUrl(databaseUrl); - - const item = await databaseArchiveFetcher( - databaseUrl, - {}, - databaseManager, - storagePath, - undefined, - { - type: "url", - url: databaseUrl, - }, - progress, - cli, - ); - - if (item) { - await commandManager.execute("codeQLDatabases.focus"); - void showAndLogInformationMessage( - extLogger, - "Database downloaded and imported successfully.", + this.validateUrl(databaseUrl); + + const item = await this.databaseArchiveFetcher( + databaseUrl, + {}, + databaseManager, + storagePath, + undefined, + { + type: "url", + url: databaseUrl, + }, + progress, + cli, ); - } - return item; -} -/** - * Prompts a user to fetch a database from GitHub. - * User enters a GitHub repository and then the user is asked which language - * to download (if there is more than one) - * - * @param app the App - * @param databaseManager the DatabaseManager - * @param storagePath where to store the unzipped database. - * @param progress the progress callback - * @param cli the CodeQL CLI server - * @param language the language to download. If undefined, the user will be prompted to choose a language. - * @param makeSelected make the new database selected in the databases panel (default: true) - * @param addSourceArchiveFolder whether to add a workspace folder containing the source archive to the workspace - */ -export async function promptImportGithubDatabase( - app: App, - databaseManager: DatabaseManager, - storagePath: string, - progress: ProgressCallback, - cli: CodeQLCliServer, - language?: string, - makeSelected = true, - addSourceArchiveFolder = addDatabaseSourceToWorkspace(), -): Promise { - const githubRepo = await askForGitHubRepo(progress); - if (!githubRepo) { - return; + if (item) { + await commandManager.execute("codeQLDatabases.focus"); + void showAndLogInformationMessage( + extLogger, + "Database downloaded and imported successfully.", + ); + } + return item; } - const databaseItem = await downloadGitHubDatabase( - githubRepo, - app, - databaseManager, - storagePath, - progress, - cli, - language, - makeSelected, - addSourceArchiveFolder, - ); - - if (databaseItem) { - if (makeSelected) { - await app.commands.execute("codeQLDatabases.focus"); + /** + * Prompts a user to fetch a database from GitHub. + * User enters a GitHub repository and then the user is asked which language + * to download (if there is more than one) + * + * @param app the App + * @param databaseManager the DatabaseManager + * @param storagePath where to store the unzipped database. + * @param progress the progress callback + * @param cli the CodeQL CLI server + * @param language the language to download. If undefined, the user will be prompted to choose a language. + * @param makeSelected make the new database selected in the databases panel (default: true) + * @param addSourceArchiveFolder whether to add a workspace folder containing the source archive to the workspace + */ + public async promptImportGithubDatabase( + app: App, + databaseManager: DatabaseManager, + storagePath: string, + progress: ProgressCallback, + cli: CodeQLCliServer, + language?: string, + makeSelected = true, + addSourceArchiveFolder = addDatabaseSourceToWorkspace(), + ): Promise { + const githubRepo = await this.askForGitHubRepo(progress); + if (!githubRepo) { + return; } - void showAndLogInformationMessage( - extLogger, - "Database downloaded and imported successfully.", + + const databaseItem = await this.downloadGitHubDatabase( + githubRepo, + app, + databaseManager, + storagePath, + progress, + cli, + language, + makeSelected, + addSourceArchiveFolder, ); - return databaseItem; - } - return; -} + if (databaseItem) { + if (makeSelected) { + await app.commands.execute("codeQLDatabases.focus"); + } + void showAndLogInformationMessage( + extLogger, + "Database downloaded and imported successfully.", + ); + return databaseItem; + } -export async function askForGitHubRepo( - progress?: ProgressCallback, - suggestedValue?: string, -): Promise { - progress?.({ - message: "Choose repository", - step: 1, - maxStep: 2, - }); - - const options: InputBoxOptions = { - title: - 'Enter a GitHub repository URL or "name with owner" (e.g. https://github.com/github/codeql or github/codeql)', - placeHolder: "https://github.com// or /", - ignoreFocusOut: true, - }; - - if (suggestedValue) { - options.value = suggestedValue; + return; } - return await window.showInputBox(options); -} + public async askForGitHubRepo( + progress?: ProgressCallback, + suggestedValue?: string, + ): Promise { + progress?.({ + message: "Choose repository", + step: 1, + maxStep: 2, + }); + + const options: InputBoxOptions = { + title: + 'Enter a GitHub repository URL or "name with owner" (e.g. https://github.com/github/codeql or github/codeql)', + placeHolder: "https://github.com// or /", + ignoreFocusOut: true, + }; + + if (suggestedValue) { + options.value = suggestedValue; + } -/** - * Downloads a database from GitHub - * - * @param githubRepo the GitHub repository to download the database from - * @param app the App - * @param databaseManager the DatabaseManager - * @param storagePath where to store the unzipped database. - * @param progress the progress callback - * @param cli the CodeQL CLI server - * @param language the language to download. If undefined, the user will be prompted to choose a language. - * @param makeSelected make the new database selected in the databases panel (default: true) - * @param addSourceArchiveFolder whether to add a workspace folder containing the source archive to the workspace - **/ -export async function downloadGitHubDatabase( - githubRepo: string, - app: App, - databaseManager: DatabaseManager, - storagePath: string, - progress: ProgressCallback, - cli: CodeQLCliServer, - language?: string, - makeSelected = true, - addSourceArchiveFolder = addDatabaseSourceToWorkspace(), -): Promise { - const nwo = getNwoFromGitHubUrl(githubRepo) || githubRepo; - if (!isValidGitHubNwo(nwo)) { - throw new Error(`Invalid GitHub repository: ${githubRepo}`); + return await window.showInputBox(options); } - const credentials = isCanary() ? app.credentials : undefined; + /** + * Downloads a database from GitHub + * + * @param githubRepo the GitHub repository to download the database from + * @param app the App + * @param databaseManager the DatabaseManager + * @param storagePath where to store the unzipped database. + * @param progress the progress callback + * @param cli the CodeQL CLI server + * @param language the language to download. If undefined, the user will be prompted to choose a language. + * @param makeSelected make the new database selected in the databases panel (default: true) + * @param addSourceArchiveFolder whether to add a workspace folder containing the source archive to the workspace + **/ + public async downloadGitHubDatabase( + githubRepo: string, + app: App, + databaseManager: DatabaseManager, + storagePath: string, + progress: ProgressCallback, + cli: CodeQLCliServer, + language?: string, + makeSelected = true, + addSourceArchiveFolder = addDatabaseSourceToWorkspace(), + ): Promise { + const nwo = getNwoFromGitHubUrl(githubRepo) || githubRepo; + if (!isValidGitHubNwo(nwo)) { + throw new Error(`Invalid GitHub repository: ${githubRepo}`); + } - const octokit = credentials - ? await credentials.getOctokit() - : new AppOctokit(); + const credentials = isCanary() ? app.credentials : undefined; - const result = await convertGithubNwoToDatabaseUrl( - nwo, - octokit, - progress, - language, - ); - if (!result) { - return; - } + const octokit = credentials + ? await credentials.getOctokit() + : new AppOctokit(); - const { databaseUrl, name, owner, databaseId, databaseCreatedAt, commitOid } = - result; - - return downloadGitHubDatabaseFromUrl( - databaseUrl, - databaseId, - databaseCreatedAt, - commitOid, - owner, - name, - octokit, - progress, - databaseManager, - storagePath, - cli, - makeSelected, - addSourceArchiveFolder, - ); -} + const result = await convertGithubNwoToDatabaseUrl( + nwo, + octokit, + progress, + language, + ); + if (!result) { + return; + } -export async function downloadGitHubDatabaseFromUrl( - databaseUrl: string, - databaseId: number, - databaseCreatedAt: string, - commitOid: string | null, - owner: string, - name: string, - octokit: Octokit, - progress: ProgressCallback, - databaseManager: DatabaseManager, - storagePath: string, - cli: CodeQLCliServer, - makeSelected = true, - addSourceArchiveFolder = true, -): Promise { - /** - * The 'token' property of the token object returned by `octokit.auth()`. - * The object is undocumented, but looks something like this: - * { - * token: 'xxxx', - * tokenType: 'oauth', - * type: 'token', - * } - * We only need the actual token string. - */ - const octokitToken = ((await octokit.auth()) as { token: string })?.token; - return await databaseArchiveFetcher( - databaseUrl, - { - Accept: "application/zip", - Authorization: octokitToken ? `Bearer ${octokitToken}` : "", - }, - databaseManager, - storagePath, - `${owner}/${name}`, - { - type: "github", - repository: `${owner}/${name}`, + const { + databaseUrl, + name, + owner, databaseId, databaseCreatedAt, commitOid, - }, - progress, - cli, - makeSelected, - addSourceArchiveFolder, - ); -} + } = result; -/** - * Imports a database from a local archive. - * - * @param databaseUrl the file url of the archive to import - * @param databaseManager the DatabaseManager - * @param storagePath where to store the unzipped database. - * @param cli the CodeQL CLI server - */ -export async function importArchiveDatabase( - commandManager: AppCommandManager, - databaseUrl: string, - databaseManager: DatabaseManager, - storagePath: string, - progress: ProgressCallback, - cli: CodeQLCliServer, -): Promise { - try { - const item = await databaseArchiveFetcher( + return this.downloadGitHubDatabaseFromUrl( databaseUrl, - {}, + databaseId, + databaseCreatedAt, + commitOid, + owner, + name, + octokit, + progress, databaseManager, storagePath, - undefined, + cli, + makeSelected, + addSourceArchiveFolder, + ); + } + + public async downloadGitHubDatabaseFromUrl( + databaseUrl: string, + databaseId: number, + databaseCreatedAt: string, + commitOid: string | null, + owner: string, + name: string, + octokit: Octokit, + progress: ProgressCallback, + databaseManager: DatabaseManager, + storagePath: string, + cli: CodeQLCliServer, + makeSelected = true, + addSourceArchiveFolder = true, + ): Promise { + /** + * The 'token' property of the token object returned by `octokit.auth()`. + * The object is undocumented, but looks something like this: + * { + * token: 'xxxx', + * tokenType: 'oauth', + * type: 'token', + * } + * We only need the actual token string. + */ + const octokitToken = ((await octokit.auth()) as { token: string })?.token; + return await this.databaseArchiveFetcher( + databaseUrl, + { + Accept: "application/zip", + Authorization: octokitToken ? `Bearer ${octokitToken}` : "", + }, + databaseManager, + storagePath, + `${owner}/${name}`, { - type: "archive", - path: databaseUrl, + type: "github", + repository: `${owner}/${name}`, + databaseId, + databaseCreatedAt, + commitOid, }, progress, cli, + makeSelected, + addSourceArchiveFolder, ); - if (item) { - await commandManager.execute("codeQLDatabases.focus"); - void showAndLogInformationMessage( - extLogger, - "Database unzipped and imported successfully.", - ); - } - return item; - } catch (e) { - if (getErrorMessage(e).includes("unexpected end of file")) { - throw new Error( - "Database is corrupt or too large. Try unzipping outside of VS Code and importing the unzipped folder instead.", - ); - } else { - // delegate - throw e; - } } -} -/** - * Fetches an archive database. The database might be on the internet - * or in the local filesystem. - * - * @param databaseUrl URL from which to grab the database - * @param requestHeaders Headers to send with the request - * @param databaseManager the DatabaseManager - * @param storagePath where to store the unzipped database. - * @param nameOverride a name for the database that overrides the default - * @param origin the origin of the database - * @param progress callback to send progress messages to - * @param cli the CodeQL CLI server - * @param makeSelected make the new database selected in the databases panel (default: true) - * @param addSourceArchiveFolder whether to add a workspace folder containing the source archive to the workspace - */ -async function databaseArchiveFetcher( - databaseUrl: string, - requestHeaders: { [key: string]: string }, - databaseManager: DatabaseManager, - storagePath: string, - nameOverride: string | undefined, - origin: DatabaseOrigin, - progress: ProgressCallback, - cli: CodeQLCliServer, - makeSelected = true, - addSourceArchiveFolder = addDatabaseSourceToWorkspace(), -): Promise { - progress({ - message: "Getting database", - step: 1, - maxStep: 4, - }); - if (!storagePath) { - throw new Error("No storage path specified."); - } - await ensureDir(storagePath); - const unzipPath = await getStorageFolder( - storagePath, - databaseUrl, - nameOverride, - ); - - if (isFile(databaseUrl)) { - await readAndUnzip(databaseUrl, unzipPath, cli, progress); - } else { - await fetchAndUnzip(databaseUrl, requestHeaders, unzipPath, cli, progress); + /** + * Imports a database from a local archive. + * + * @param databaseUrl the file url of the archive to import + * @param databaseManager the DatabaseManager + * @param storagePath where to store the unzipped database. + * @param cli the CodeQL CLI server + */ + public async importArchiveDatabase( + commandManager: AppCommandManager, + databaseUrl: string, + databaseManager: DatabaseManager, + storagePath: string, + progress: ProgressCallback, + cli: CodeQLCliServer, + ): Promise { + try { + const item = await this.databaseArchiveFetcher( + databaseUrl, + {}, + databaseManager, + storagePath, + undefined, + { + type: "archive", + path: databaseUrl, + }, + progress, + cli, + ); + if (item) { + await commandManager.execute("codeQLDatabases.focus"); + void showAndLogInformationMessage( + extLogger, + "Database unzipped and imported successfully.", + ); + } + return item; + } catch (e) { + if (getErrorMessage(e).includes("unexpected end of file")) { + throw new Error( + "Database is corrupt or too large. Try unzipping outside of VS Code and importing the unzipped folder instead.", + ); + } else { + // delegate + throw e; + } + } } - progress({ - message: "Opening database", - step: 3, - maxStep: 4, - }); - - // find the path to the database. The actual database might be in a sub-folder - const dbPath = await findDirWithFile( - unzipPath, - ".dbinfo", - "codeql-database.yml", - ); - if (dbPath) { + /** + * Fetches an archive database. The database might be on the internet + * or in the local filesystem. + * + * @param databaseUrl URL from which to grab the database + * @param requestHeaders Headers to send with the request + * @param databaseManager the DatabaseManager + * @param storagePath where to store the unzipped database. + * @param nameOverride a name for the database that overrides the default + * @param origin the origin of the database + * @param progress callback to send progress messages to + * @param cli the CodeQL CLI server + * @param makeSelected make the new database selected in the databases panel (default: true) + * @param addSourceArchiveFolder whether to add a workspace folder containing the source archive to the workspace + */ + private async databaseArchiveFetcher( + databaseUrl: string, + requestHeaders: { [key: string]: string }, + databaseManager: DatabaseManager, + storagePath: string, + nameOverride: string | undefined, + origin: DatabaseOrigin, + progress: ProgressCallback, + cli: CodeQLCliServer, + makeSelected = true, + addSourceArchiveFolder = addDatabaseSourceToWorkspace(), + ): Promise { progress({ - message: "Validating and fixing source location", - step: 4, + message: "Getting database", + step: 1, maxStep: 4, }); - await ensureZippedSourceLocation(dbPath); - - const item = await databaseManager.openDatabase( - Uri.file(dbPath), - origin, - makeSelected, + if (!storagePath) { + throw new Error("No storage path specified."); + } + await ensureDir(storagePath); + const unzipPath = await this.getStorageFolder( + storagePath, + databaseUrl, nameOverride, - { - addSourceArchiveFolder, - extensionManagedLocation: unzipPath, - }, ); - return item; - } else { - throw new Error("Database not found in archive."); - } -} -// The number of tries to use when generating a unique filename before -// giving up and using a nanoid. -const DUPLICATE_FILENAMES_TRIES = 10_000; + if (this.isFile(databaseUrl)) { + await this.readAndUnzip(databaseUrl, unzipPath, cli, progress); + } else { + await this.fetchAndUnzip( + databaseUrl, + requestHeaders, + unzipPath, + cli, + progress, + ); + } + + progress({ + message: "Opening database", + step: 3, + maxStep: 4, + }); -async function getStorageFolder( - storagePath: string, - urlStr: string, - nameOverrride?: string, -) { - let lastName: string; - - if (nameOverrride) { - lastName = createFilenameFromString(nameOverrride); - } else { - // we need to generate a folder name for the unzipped archive, - // this needs to be human readable since we may use this name as the initial - // name for the database - const url = Uri.parse(urlStr); - // MacOS has a max filename length of 255 - // and remove a few extra chars in case we need to add a counter at the end. - lastName = basename(url.path).substring(0, 250); - if (lastName.endsWith(".zip")) { - lastName = lastName.substring(0, lastName.length - 4); + // find the path to the database. The actual database might be in a sub-folder + const dbPath = await findDirWithFile( + unzipPath, + ".dbinfo", + "codeql-database.yml", + ); + if (dbPath) { + progress({ + message: "Validating and fixing source location", + step: 4, + maxStep: 4, + }); + await this.ensureZippedSourceLocation(dbPath); + + const item = await databaseManager.openDatabase( + Uri.file(dbPath), + origin, + makeSelected, + nameOverride, + { + addSourceArchiveFolder, + extensionManagedLocation: unzipPath, + }, + ); + return item; + } else { + throw new Error("Database not found in archive."); } } - const realpath = await fs_realpath(storagePath); - let folderName = lastName; - - // get all existing files instead of calling pathExists on every - // single combination of realpath and folderName - const existingFiles = await readdir(realpath); - - // avoid overwriting existing folders - let counter = 0; - while (existingFiles.includes(basename(folderName))) { - counter++; - - if (counter <= DUPLICATE_FILENAMES_TRIES) { - // First try to use a counter to make the name unique. - folderName = `${lastName}-${counter}`; - } else if (counter <= DUPLICATE_FILENAMES_TRIES + 5) { - // If there are more than 10,000 similarly named databases, - // give up on using a counter and use a random string instead. - folderName = `${lastName}-${nanoid()}`; + private async getStorageFolder( + storagePath: string, + urlStr: string, + nameOverrride?: string, + ) { + let lastName: string; + + if (nameOverrride) { + lastName = createFilenameFromString(nameOverrride); } else { - // This should almost never happen, but just in case, we don't want to - // get stuck in an infinite loop. - throw new Error( - "Could not find a unique name for downloaded database. Please remove some databases and try again.", - ); + // we need to generate a folder name for the unzipped archive, + // this needs to be human readable since we may use this name as the initial + // name for the database + const url = Uri.parse(urlStr); + // MacOS has a max filename length of 255 + // and remove a few extra chars in case we need to add a counter at the end. + lastName = basename(url.path).substring(0, 250); + if (lastName.endsWith(".zip")) { + lastName = lastName.substring(0, lastName.length - 4); + } } - } - return join(realpath, folderName); -} -function validateUrl(databaseUrl: string) { - let uri; - try { - uri = Uri.parse(databaseUrl, true); - } catch (e) { - throw new Error(`Invalid url: ${databaseUrl}`); + const realpath = await fs_realpath(storagePath); + let folderName = lastName; + + // get all existing files instead of calling pathExists on every + // single combination of realpath and folderName + const existingFiles = await readdir(realpath); + + // avoid overwriting existing folders + let counter = 0; + while (existingFiles.includes(basename(folderName))) { + counter++; + + if (counter <= DUPLICATE_FILENAMES_TRIES) { + // First try to use a counter to make the name unique. + folderName = `${lastName}-${counter}`; + } else if (counter <= DUPLICATE_FILENAMES_TRIES + 5) { + // If there are more than 10,000 similarly named databases, + // give up on using a counter and use a random string instead. + folderName = `${lastName}-${nanoid()}`; + } else { + // This should almost never happen, but just in case, we don't want to + // get stuck in an infinite loop. + throw new Error( + "Could not find a unique name for downloaded database. Please remove some databases and try again.", + ); + } + } + return join(realpath, folderName); } - if (!allowHttp() && uri.scheme !== "https") { - throw new Error("Must use https for downloading a database."); + private validateUrl(databaseUrl: string) { + let uri; + try { + uri = Uri.parse(databaseUrl, true); + } catch (e) { + throw new Error(`Invalid url: ${databaseUrl}`); + } + + if (!allowHttp() && uri.scheme !== "https") { + throw new Error("Must use https for downloading a database."); + } } -} -async function readAndUnzip( - zipUrl: string, - unzipPath: string, - cli: CodeQLCliServer, - progress?: ProgressCallback, -) { - const zipFile = Uri.parse(zipUrl).fsPath; - progress?.({ - maxStep: 10, - step: 9, - message: `Unzipping into ${basename(unzipPath)}`, - }); - - await cli.databaseUnbundle(zipFile, unzipPath); -} + private async readAndUnzip( + zipUrl: string, + unzipPath: string, + cli: CodeQLCliServer, + progress?: ProgressCallback, + ) { + const zipFile = Uri.parse(zipUrl).fsPath; + progress?.({ + maxStep: 10, + step: 9, + message: `Unzipping into ${basename(unzipPath)}`, + }); -async function fetchAndUnzip( - databaseUrl: string, - requestHeaders: { [key: string]: string }, - unzipPath: string, - cli: CodeQLCliServer, - progress?: ProgressCallback, -) { - // Although it is possible to download and stream directly to an unzipped directory, - // we need to avoid this for two reasons. The central directory is located at the - // end of the zip file. It is the source of truth of the content locations. Individual - // file headers may be incorrect. Additionally, saving to file first will reduce memory - // pressure compared with unzipping while downloading the archive. - - const archivePath = join(tmpDir.name, `archive-${Date.now()}.zip`); - - progress?.({ - maxStep: 3, - message: "Downloading database", - step: 1, - }); - - const { - signal, - onData, - dispose: disposeTimeout, - } = createTimeoutSignal(downloadTimeout()); - - let response: Response; - try { - response = await checkForFailingResponse( - await fetch(databaseUrl, { - headers: requestHeaders, - signal, - }), - "Error downloading database", - ); - } catch (e) { - disposeTimeout(); + await cli.databaseUnbundle(zipFile, unzipPath); + } - if (e instanceof AbortError) { - const thrownError = new AbortError("The request timed out."); - thrownError.stack = e.stack; - throw thrownError; - } + private async fetchAndUnzip( + databaseUrl: string, + requestHeaders: { [key: string]: string }, + unzipPath: string, + cli: CodeQLCliServer, + progress?: ProgressCallback, + ) { + // Although it is possible to download and stream directly to an unzipped directory, + // we need to avoid this for two reasons. The central directory is located at the + // end of the zip file. It is the source of truth of the content locations. Individual + // file headers may be incorrect. Additionally, saving to file first will reduce memory + // pressure compared with unzipping while downloading the archive. + + const archivePath = join(tmpDir.name, `archive-${Date.now()}.zip`); + + progress?.({ + maxStep: 3, + message: "Downloading database", + step: 1, + }); - throw e; - } + const { + signal, + onData, + dispose: disposeTimeout, + } = createTimeoutSignal(downloadTimeout()); + + let response: Response; + try { + response = await this.checkForFailingResponse( + await fetch(databaseUrl, { + headers: requestHeaders, + signal, + }), + "Error downloading database", + ); + } catch (e) { + disposeTimeout(); - const archiveFileStream = createWriteStream(archivePath); + if (e instanceof AbortError) { + const thrownError = new AbortError("The request timed out."); + thrownError.stack = e.stack; + throw thrownError; + } - const contentLength = response.headers.get("content-length"); - const totalNumBytes = contentLength ? parseInt(contentLength, 10) : undefined; - reportStreamProgress( - response.body, - "Downloading database", - totalNumBytes, - progress, - ); + throw e; + } - response.body.on("data", onData); + const archiveFileStream = createWriteStream(archivePath); - try { - await new Promise((resolve, reject) => { - response.body - .pipe(archiveFileStream) - .on("finish", resolve) - .on("error", reject); + const contentLength = response.headers.get("content-length"); + const totalNumBytes = contentLength + ? parseInt(contentLength, 10) + : undefined; + reportStreamProgress( + response.body, + "Downloading database", + totalNumBytes, + progress, + ); - // If an error occurs on the body, we also want to reject the promise (e.g. during a timeout error). - response.body.on("error", reject); - }); - } catch (e) { - // Close and remove the file if an error occurs - archiveFileStream.close(() => { - void remove(archivePath); - }); + response.body.on("data", onData); + + try { + await new Promise((resolve, reject) => { + response.body + .pipe(archiveFileStream) + .on("finish", resolve) + .on("error", reject); + + // If an error occurs on the body, we also want to reject the promise (e.g. during a timeout error). + response.body.on("error", reject); + }); + } catch (e) { + // Close and remove the file if an error occurs + archiveFileStream.close(() => { + void remove(archivePath); + }); + + if (e instanceof AbortError) { + const thrownError = new AbortError("The download timed out."); + thrownError.stack = e.stack; + throw thrownError; + } - if (e instanceof AbortError) { - const thrownError = new AbortError("The download timed out."); - thrownError.stack = e.stack; - throw thrownError; + throw e; + } finally { + disposeTimeout(); } - throw e; - } finally { - disposeTimeout(); - } + await this.readAndUnzip( + Uri.file(archivePath).toString(true), + unzipPath, + cli, + progress, + ); - await readAndUnzip( - Uri.file(archivePath).toString(true), - unzipPath, - cli, - progress, - ); + // remove archivePath eagerly since these archives can be large. + await remove(archivePath); + } - // remove archivePath eagerly since these archives can be large. - await remove(archivePath); -} + private async checkForFailingResponse( + response: Response, + errorMessage: string, + ): Promise { + if (response.ok) { + return response; + } -async function checkForFailingResponse( - response: Response, - errorMessage: string, -): Promise { - if (response.ok) { - return response; + // An error downloading the database. Attempt to extract the reason behind it. + const text = await response.text(); + let msg: string; + try { + const obj = JSON.parse(text); + msg = + obj.error || obj.message || obj.reason || JSON.stringify(obj, null, 2); + } catch (e) { + msg = text; + } + throw new Error(`${errorMessage}.\n\nReason: ${msg}`); } - // An error downloading the database. Attempt to extract the reason behind it. - const text = await response.text(); - let msg: string; - try { - const obj = JSON.parse(text); - msg = - obj.error || obj.message || obj.reason || JSON.stringify(obj, null, 2); - } catch (e) { - msg = text; + private isFile(databaseUrl: string) { + return Uri.parse(databaseUrl).scheme === "file"; } - throw new Error(`${errorMessage}.\n\nReason: ${msg}`); -} -function isFile(databaseUrl: string) { - return Uri.parse(databaseUrl).scheme === "file"; -} - -/** - * Databases created by the old odasa tool will not have a zipped - * source location. However, this extension works better if sources - * are zipped. - * - * This function ensures that the source location is zipped. If the - * `src` folder exists and the `src.zip` file does not, the `src` - * folder will be zipped and then deleted. - * - * @param databasePath The full path to the unzipped database - */ -async function ensureZippedSourceLocation(databasePath: string): Promise { - const srcFolderPath = join(databasePath, "src"); - const srcZipPath = `${srcFolderPath}.zip`; - - if ((await pathExists(srcFolderPath)) && !(await pathExists(srcZipPath))) { - await zip(srcFolderPath, srcZipPath); - await remove(srcFolderPath); + /** + * Databases created by the old odasa tool will not have a zipped + * source location. However, this extension works better if sources + * are zipped. + * + * This function ensures that the source location is zipped. If the + * `src` folder exists and the `src.zip` file does not, the `src` + * folder will be zipped and then deleted. + * + * @param databasePath The full path to the unzipped database + */ + private async ensureZippedSourceLocation( + databasePath: string, + ): Promise { + const srcFolderPath = join(databasePath, "src"); + const srcZipPath = `${srcFolderPath}.zip`; + + if ((await pathExists(srcFolderPath)) && !(await pathExists(srcZipPath))) { + await zip(srcFolderPath, srcZipPath); + await remove(srcFolderPath); + } } } diff --git a/extensions/ql-vscode/src/databases/github-databases/download.ts b/extensions/ql-vscode/src/databases/github-databases/download.ts index 21a2e12231a..89dc7527e89 100644 --- a/extensions/ql-vscode/src/databases/github-databases/download.ts +++ b/extensions/ql-vscode/src/databases/github-databases/download.ts @@ -2,7 +2,7 @@ import { window } from "vscode"; import type { Octokit } from "@octokit/rest"; import { showNeverAskAgainDialog } from "../../common/vscode/dialog"; import { getLanguageDisplayName } from "../../common/query-language"; -import { downloadGitHubDatabaseFromUrl } from "../database-fetcher"; +import type { DatabaseFetcher } from "../database-fetcher"; import { withProgress } from "../../common/vscode/progress"; import type { DatabaseManager } from "../local-databases"; import type { CodeQLCliServer } from "../../codeql-cli/cli"; @@ -59,6 +59,7 @@ export async function downloadDatabaseFromGitHub( repo: string, databases: CodeqlDatabase[], databaseManager: DatabaseManager, + databaseFetcher: DatabaseFetcher, storagePath: string, cliServer: CodeQLCliServer, commandManager: AppCommandManager, @@ -72,7 +73,7 @@ export async function downloadDatabaseFromGitHub( selectedDatabases.map((database) => withProgress( async (progress) => { - await downloadGitHubDatabaseFromUrl( + await databaseFetcher.downloadGitHubDatabaseFromUrl( database.url, database.id, database.created_at, diff --git a/extensions/ql-vscode/src/databases/github-databases/github-databases-module.ts b/extensions/ql-vscode/src/databases/github-databases/github-databases-module.ts index 1b89840e854..382d638fa42 100644 --- a/extensions/ql-vscode/src/databases/github-databases/github-databases-module.ts +++ b/extensions/ql-vscode/src/databases/github-databases/github-databases-module.ts @@ -24,6 +24,7 @@ import { isNewerDatabaseAvailable, } from "./updates"; import type { Octokit } from "@octokit/rest"; +import type { DatabaseFetcher } from "../database-fetcher"; export class GitHubDatabasesModule extends DisposableObject { /** @@ -33,6 +34,7 @@ export class GitHubDatabasesModule extends DisposableObject { constructor( private readonly app: App, private readonly databaseManager: DatabaseManager, + private readonly databaseFetcher: DatabaseFetcher, private readonly databaseStoragePath: string, private readonly cliServer: CodeQLCliServer, private readonly config: GitHubDatabaseConfig, @@ -43,6 +45,7 @@ export class GitHubDatabasesModule extends DisposableObject { public static async initialize( app: App, databaseManager: DatabaseManager, + databaseFetcher: DatabaseFetcher, databaseStoragePath: string, cliServer: CodeQLCliServer, config: GitHubDatabaseConfig, @@ -50,6 +53,7 @@ export class GitHubDatabasesModule extends DisposableObject { const githubDatabasesModule = new GitHubDatabasesModule( app, databaseManager, + databaseFetcher, databaseStoragePath, cliServer, config, @@ -186,6 +190,7 @@ export class GitHubDatabasesModule extends DisposableObject { repo, databases, this.databaseManager, + this.databaseFetcher, this.databaseStoragePath, this.cliServer, this.app.commands, @@ -212,6 +217,7 @@ export class GitHubDatabasesModule extends DisposableObject { repo, databaseUpdates, this.databaseManager, + this.databaseFetcher, this.databaseStoragePath, this.cliServer, this.app.commands, diff --git a/extensions/ql-vscode/src/databases/github-databases/updates.ts b/extensions/ql-vscode/src/databases/github-databases/updates.ts index 317870f5c41..349b317ce04 100644 --- a/extensions/ql-vscode/src/databases/github-databases/updates.ts +++ b/extensions/ql-vscode/src/databases/github-databases/updates.ts @@ -5,7 +5,7 @@ import type { CodeQLCliServer } from "../../codeql-cli/cli"; import type { AppCommandManager } from "../../common/commands"; import { getLanguageDisplayName } from "../../common/query-language"; import { showNeverAskAgainDialog } from "../../common/vscode/dialog"; -import { downloadGitHubDatabaseFromUrl } from "../database-fetcher"; +import type { DatabaseFetcher } from "../database-fetcher"; import { withProgress } from "../../common/vscode/progress"; import { window } from "vscode"; import type { GitHubDatabaseConfig } from "../../config"; @@ -156,6 +156,7 @@ export async function downloadDatabaseUpdateFromGitHub( repo: string, updates: DatabaseUpdate[], databaseManager: DatabaseManager, + databaseFetcher: DatabaseFetcher, storagePath: string, cliServer: CodeQLCliServer, commandManager: AppCommandManager, @@ -179,21 +180,22 @@ export async function downloadDatabaseUpdateFromGitHub( return withProgress( async (progress) => { - const newDatabase = await downloadGitHubDatabaseFromUrl( - database.url, - database.id, - database.created_at, - database.commit_oid ?? null, - owner, - repo, - octokit, - progress, - databaseManager, - storagePath, - cliServer, - databaseManager.currentDatabaseItem === update.databaseItem, - update.databaseItem.hasSourceArchiveInExplorer(), - ); + const newDatabase = + await databaseFetcher.downloadGitHubDatabaseFromUrl( + database.url, + database.id, + database.created_at, + database.commit_oid ?? null, + owner, + repo, + octokit, + progress, + databaseManager, + storagePath, + cliServer, + databaseManager.currentDatabaseItem === update.databaseItem, + update.databaseItem.hasSourceArchiveInExplorer(), + ); if (newDatabase === undefined) { return; } diff --git a/extensions/ql-vscode/src/databases/local-databases-ui.ts b/extensions/ql-vscode/src/databases/local-databases-ui.ts index c5230ce72cb..59cfb4d7206 100644 --- a/extensions/ql-vscode/src/databases/local-databases-ui.ts +++ b/extensions/ql-vscode/src/databases/local-databases-ui.ts @@ -42,11 +42,7 @@ import { showAndLogExceptionWithTelemetry, showAndLogErrorMessage, } from "../common/logging"; -import { - importArchiveDatabase, - promptImportGithubDatabase, - promptImportInternetDatabase, -} from "./database-fetcher"; +import { DatabaseFetcher } from "./database-fetcher"; import { asError, asyncFilter, getErrorMessage } from "../common/helpers-pure"; import type { QueryRunner } from "../query-server"; import type { App } from "../common/app"; @@ -540,7 +536,7 @@ export class DatabaseUI extends DisposableObject { private async handleChooseDatabaseInternet(): Promise { return withProgress( async (progress) => { - await promptImportInternetDatabase( + await new DatabaseFetcher().promptImportInternetDatabase( this.app.commands, this.databaseManager, this.storagePath, @@ -557,7 +553,7 @@ export class DatabaseUI extends DisposableObject { private async handleChooseDatabaseGithub(): Promise { return withProgress( async (progress) => { - await promptImportGithubDatabase( + await new DatabaseFetcher().promptImportGithubDatabase( this.app, this.databaseManager, this.storagePath, @@ -712,7 +708,7 @@ export class DatabaseUI extends DisposableObject { try { // Assume user has selected an archive if the file has a .zip extension if (uri.path.endsWith(".zip")) { - await importArchiveDatabase( + await new DatabaseFetcher().importArchiveDatabase( this.app.commands, uri.toString(true), this.databaseManager, @@ -959,7 +955,7 @@ export class DatabaseUI extends DisposableObject { } else { // we are selecting a database archive. Must unzip into a workspace-controlled area // before importing. - return await importArchiveDatabase( + return await new DatabaseFetcher().importArchiveDatabase( this.app.commands, uri.toString(true), this.databaseManager, diff --git a/extensions/ql-vscode/src/extension.ts b/extensions/ql-vscode/src/extension.ts index 53a3b12dfd4..5658788e582 100644 --- a/extensions/ql-vscode/src/extension.ts +++ b/extensions/ql-vscode/src/extension.ts @@ -133,6 +133,7 @@ import { OpenReferencedFileCodeLensProvider } from "./local-queries/open-referen import { LanguageContextStore } from "./language-context-store"; import { LanguageSelectionPanel } from "./language-selection-panel/language-selection-panel"; import { GitHubDatabasesModule } from "./databases/github-databases"; +import { DatabaseFetcher } from "./databases/database-fetcher"; /** * extension.ts @@ -881,6 +882,7 @@ async function activateWithInstalledDistribution( await GitHubDatabasesModule.initialize( app, dbm, + new DatabaseFetcher(), getContextStoragePath(ctx), cliServer, githubDatabaseConfigListener, diff --git a/extensions/ql-vscode/src/local-queries/local-queries.ts b/extensions/ql-vscode/src/local-queries/local-queries.ts index ddc29db949b..ea1c5f8e864 100644 --- a/extensions/ql-vscode/src/local-queries/local-queries.ts +++ b/extensions/ql-vscode/src/local-queries/local-queries.ts @@ -51,6 +51,7 @@ import type { QueryTreeViewItem } from "../queries-panel/query-tree-view-item"; import { tryGetQueryLanguage } from "../common/query-language"; import type { LanguageContextStore } from "../language-context-store"; import type { ExtensionApp } from "../common/vscode/vscode-app"; +import { DatabaseFetcher } from "../databases/database-fetcher"; export enum QuickEvalType { None, @@ -330,6 +331,7 @@ export class LocalQueries extends DisposableObject { progress, this.app, this.databaseManager, + new DatabaseFetcher(), contextStoragePath, this.selectedQueryTreeViewItems, language, diff --git a/extensions/ql-vscode/src/local-queries/skeleton-query-wizard.ts b/extensions/ql-vscode/src/local-queries/skeleton-query-wizard.ts index 0079709bbf6..5305fe40a01 100644 --- a/extensions/ql-vscode/src/local-queries/skeleton-query-wizard.ts +++ b/extensions/ql-vscode/src/local-queries/skeleton-query-wizard.ts @@ -19,10 +19,7 @@ import { UserCancellationException, withProgress, } from "../common/vscode/progress"; -import { - askForGitHubRepo, - downloadGitHubDatabase, -} from "../databases/database-fetcher"; +import type { DatabaseFetcher } from "../databases/database-fetcher"; import { getQlPackLocation, isCodespacesTemplate, @@ -62,6 +59,7 @@ export class SkeletonQueryWizard { private readonly progress: ProgressCallback, private readonly app: App, private readonly databaseManager: DatabaseManager, + private readonly databaseFetcher: DatabaseFetcher, private readonly databaseStoragePath: string | undefined, private readonly selectedItems: readonly QueryTreeViewItem[], private language: QueryLanguage | undefined = undefined, @@ -378,13 +376,16 @@ export class SkeletonQueryWizard { }); const githubRepoNwo = QUERY_LANGUAGE_TO_DATABASE_REPO[this.language]; - const chosenRepo = await askForGitHubRepo(undefined, githubRepoNwo); + const chosenRepo = await this.databaseFetcher.askForGitHubRepo( + undefined, + githubRepoNwo, + ); if (!chosenRepo) { throw new UserCancellationException("No GitHub repository provided"); } - await downloadGitHubDatabase( + await this.databaseFetcher.downloadGitHubDatabase( chosenRepo, this.app, this.databaseManager, diff --git a/extensions/ql-vscode/src/model-editor/model-editor-view.ts b/extensions/ql-vscode/src/model-editor/model-editor-view.ts index 0ad90435813..2e72981acd7 100644 --- a/extensions/ql-vscode/src/model-editor/model-editor-view.ts +++ b/extensions/ql-vscode/src/model-editor/model-editor-view.ts @@ -29,7 +29,7 @@ import type { } from "../databases/local-databases"; import type { CodeQLCliServer } from "../codeql-cli/cli"; import { asError, assertNever, getErrorMessage } from "../common/helpers-pure"; -import { promptImportGithubDatabase } from "../databases/database-fetcher"; +import { DatabaseFetcher } from "../databases/database-fetcher"; import type { App } from "../common/app"; import { redactableError } from "../common/errors"; import { @@ -916,16 +916,17 @@ export class ModelEditorView extends AbstractWebview< // the user to import the library database. We need to have the database // imported to the query server, so we need to register it to our workspace. const makeSelected = false; - const addedDatabase = await promptImportGithubDatabase( - this.app, - this.databaseManager, - this.app.workspaceStoragePath ?? this.app.globalStoragePath, - progress, - this.cliServer, - this.databaseItem.language, - makeSelected, - false, - ); + const addedDatabase = + await new DatabaseFetcher().promptImportGithubDatabase( + this.app, + this.databaseManager, + this.app.workspaceStoragePath ?? this.app.globalStoragePath, + progress, + this.cliServer, + this.databaseItem.language, + makeSelected, + false, + ); if (!addedDatabase) { void this.app.logger.log("No database chosen"); return; diff --git a/extensions/ql-vscode/test/vscode-tests/cli-integration/databases/database-fetcher.test.ts b/extensions/ql-vscode/test/vscode-tests/cli-integration/databases/database-fetcher.test.ts index ed80c1f037b..cc4b6f2d5a4 100644 --- a/extensions/ql-vscode/test/vscode-tests/cli-integration/databases/database-fetcher.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/cli-integration/databases/database-fetcher.test.ts @@ -3,10 +3,7 @@ import { Uri, window } from "vscode"; import type { CodeQLCliServer } from "../../../../src/codeql-cli/cli"; import type { DatabaseManager } from "../../../../src/databases/local-databases"; -import { - importArchiveDatabase, - promptImportInternetDatabase, -} from "../../../../src/databases/database-fetcher"; +import { DatabaseFetcher } from "../../../../src/databases/database-fetcher"; import { cleanDatabases, dbLoc, @@ -49,7 +46,7 @@ describe("database-fetcher", () => { describe("importArchiveDatabase", () => { it("should add a database from a folder", async () => { const uri = Uri.file(dbLoc); - let dbItem = await importArchiveDatabase( + let dbItem = await new DatabaseFetcher().importArchiveDatabase( createMockCommandManager(), uri.toString(true), databaseManager, @@ -71,7 +68,7 @@ describe("database-fetcher", () => { // Provide a database URL when prompted inputBoxStub.mockResolvedValue(DB_URL); - let dbItem = await promptImportInternetDatabase( + let dbItem = await new DatabaseFetcher().promptImportInternetDatabase( createMockCommandManager(), databaseManager, storagePath, diff --git a/extensions/ql-vscode/test/vscode-tests/cli-integration/local-queries/skeleton-query-wizard.test.ts b/extensions/ql-vscode/test/vscode-tests/cli-integration/local-queries/skeleton-query-wizard.test.ts index 7f4c6d8e2b3..319a0e8cfe9 100644 --- a/extensions/ql-vscode/test/vscode-tests/cli-integration/local-queries/skeleton-query-wizard.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/cli-integration/local-queries/skeleton-query-wizard.test.ts @@ -23,7 +23,7 @@ import type { DatabaseManager, FullDatabaseOptions, } from "../../../../src/databases/local-databases"; -import * as databaseFetcher from "../../../../src/databases/database-fetcher"; +import { DatabaseFetcher } from "../../../../src/databases/database-fetcher"; import { createMockDB } from "../../../factories/databases/databases"; import { asError } from "../../../../src/common/helpers-pure"; import { Setting } from "../../../../src/config"; @@ -42,6 +42,7 @@ describe("SkeletonQueryWizard", () => { let mockApp: App; let wizard: SkeletonQueryWizard; let mockDatabaseManager: DatabaseManager; + let databaseFetcher: DatabaseFetcher; let dir: DirResult; let storagePath: string; let quickPickSpy: jest.SpiedFunction; @@ -56,10 +57,10 @@ describe("SkeletonQueryWizard", () => { typeof QlPackGenerator.prototype.createExampleQlFile >; let downloadGitHubDatabaseSpy: jest.SpiedFunction< - typeof databaseFetcher.downloadGitHubDatabase + DatabaseFetcher["downloadGitHubDatabase"] >; let askForGitHubRepoSpy: jest.SpiedFunction< - typeof databaseFetcher.askForGitHubRepo + DatabaseFetcher["askForGitHubRepo"] >; let openTextDocumentSpy: jest.SpiedFunction< typeof workspace.openTextDocument @@ -115,6 +116,8 @@ describe("SkeletonQueryWizard", () => { }, ] as WorkspaceFolder[]); + databaseFetcher = new DatabaseFetcher(); + quickPickSpy = jest.spyOn(window, "showQuickPick").mockResolvedValueOnce( mockedQuickPickItem({ label: chosenLanguage, @@ -145,6 +148,7 @@ describe("SkeletonQueryWizard", () => { jest.fn(), mockApp, mockDatabaseManager, + databaseFetcher, storagePath, selectedItems, ); @@ -172,6 +176,7 @@ describe("SkeletonQueryWizard", () => { jest.fn(), mockApp, mockDatabaseManager, + databaseFetcher, storagePath, selectedItems, QueryLanguage.Swift, @@ -320,6 +325,7 @@ describe("SkeletonQueryWizard", () => { jest.fn(), mockApp, mockDatabaseManagerWithItems, + databaseFetcher, storagePath, selectedItems, ); @@ -369,6 +375,7 @@ describe("SkeletonQueryWizard", () => { jest.fn(), mockApp, mockDatabaseManagerWithItems, + databaseFetcher, storagePath, selectedItems, ); @@ -504,6 +511,7 @@ describe("SkeletonQueryWizard", () => { jest.fn(), mockApp, mockDatabaseManager, + databaseFetcher, storagePath, selectedItems, QueryLanguage.Javascript, @@ -725,6 +733,7 @@ describe("SkeletonQueryWizard", () => { jest.fn(), mockApp, mockDatabaseManager, + databaseFetcher, storagePath, selectedItems, ); @@ -754,6 +763,7 @@ describe("SkeletonQueryWizard", () => { jest.fn(), mockApp, mockDatabaseManager, + databaseFetcher, storagePath, selectedItems, ); @@ -787,6 +797,7 @@ describe("SkeletonQueryWizard", () => { jest.fn(), mockApp, mockDatabaseManager, + databaseFetcher, storagePath, selectedItems, QueryLanguage.Swift, @@ -830,6 +841,7 @@ describe("SkeletonQueryWizard", () => { jest.fn(), mockApp, mockDatabaseManager, + databaseFetcher, storagePath, selectedItems, ); diff --git a/extensions/ql-vscode/test/vscode-tests/global.helper.ts b/extensions/ql-vscode/test/vscode-tests/global.helper.ts index 36be9878915..b0f85a8ec09 100644 --- a/extensions/ql-vscode/test/vscode-tests/global.helper.ts +++ b/extensions/ql-vscode/test/vscode-tests/global.helper.ts @@ -7,7 +7,7 @@ import type { } from "../../src/databases/local-databases"; import type { CodeQLCliServer } from "../../src/codeql-cli/cli"; import type { CodeQLExtensionInterface } from "../../src/extension"; -import { importArchiveDatabase } from "../../src/databases/database-fetcher"; +import { DatabaseFetcher } from "../../src/databases/database-fetcher"; import { createMockCommandManager } from "../__mocks__/commandsMock"; // This file contains helpers shared between tests that work with an activated extension. @@ -34,7 +34,7 @@ export async function ensureTestDatabase( // Add a database, but make sure the database manager is empty first await cleanDatabases(databaseManager); const uri = Uri.file(dbLoc); - const maybeDbItem = await importArchiveDatabase( + const maybeDbItem = await new DatabaseFetcher().importArchiveDatabase( createMockCommandManager(), uri.toString(true), databaseManager, diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/download.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/download.test.ts index df2b329c12d..3768dca4992 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/download.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/download.test.ts @@ -14,7 +14,7 @@ import type { DatabaseManager } from "../../../../../src/databases/local-databas import type { GitHubDatabaseConfig } from "../../../../../src/config"; import type { CodeQLCliServer } from "../../../../../src/codeql-cli/cli"; import { createMockCommandManager } from "../../../../__mocks__/commandsMock"; -import * as databaseFetcher from "../../../../../src/databases/database-fetcher"; +import { DatabaseFetcher } from "../../../../../src/databases/database-fetcher"; import * as dialog from "../../../../../src/common/vscode/dialog"; import type { CodeqlDatabase } from "../../../../../src/databases/github-databases/api"; @@ -97,6 +97,7 @@ describe("downloadDatabaseFromGitHub", () => { const owner = "github"; const repo = "codeql"; let databaseManager: DatabaseManager; + let databaseFetcher: DatabaseFetcher; const storagePath = "/a/b/c/d"; let cliServer: CodeQLCliServer; @@ -117,12 +118,13 @@ describe("downloadDatabaseFromGitHub", () => { let showQuickPickSpy: jest.SpiedFunction; let downloadGitHubDatabaseFromUrlSpy: jest.SpiedFunction< - typeof databaseFetcher.downloadGitHubDatabaseFromUrl + DatabaseFetcher["downloadGitHubDatabaseFromUrl"] >; beforeEach(() => { octokit = mockedObject({}); databaseManager = mockedObject({}); + databaseFetcher = new DatabaseFetcher(); cliServer = mockedObject({}); showQuickPickSpy = jest.spyOn(window, "showQuickPick").mockResolvedValue( @@ -144,6 +146,7 @@ describe("downloadDatabaseFromGitHub", () => { repo, databases, databaseManager, + databaseFetcher, storagePath, cliServer, commandManager, @@ -208,6 +211,7 @@ describe("downloadDatabaseFromGitHub", () => { repo, databases, databaseManager, + databaseFetcher, storagePath, cliServer, commandManager, @@ -264,6 +268,7 @@ describe("downloadDatabaseFromGitHub", () => { repo, databases, databaseManager, + databaseFetcher, storagePath, cliServer, commandManager, @@ -329,6 +334,7 @@ describe("downloadDatabaseFromGitHub", () => { repo, databases, databaseManager, + databaseFetcher, storagePath, cliServer, commandManager, diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/github-databases-module.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/github-databases-module.test.ts index d2e605ef103..d5081a1c4d5 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/github-databases-module.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/github-databases-module.test.ts @@ -16,6 +16,7 @@ import * as githubDatabasesApi from "../../../../../src/databases/github-databas import * as githubDatabasesDownload from "../../../../../src/databases/github-databases/download"; import * as githubDatabasesUpdates from "../../../../../src/databases/github-databases/updates"; import type { DatabaseUpdate } from "../../../../../src/databases/github-databases/updates"; +import { DatabaseFetcher } from "../../../../../src/databases/database-fetcher"; describe("GitHubDatabasesModule", () => { describe("promptGitHubRepositoryDownload", () => { @@ -23,6 +24,7 @@ describe("GitHubDatabasesModule", () => { let databaseManager: DatabaseManager; let databaseStoragePath: string; let cliServer: CodeQLCliServer; + let databaseFetcher: DatabaseFetcher; let config: GitHubDatabaseConfig; let gitHubDatabasesModule: GitHubDatabasesModule; @@ -66,6 +68,7 @@ describe("GitHubDatabasesModule", () => { databaseManager = mockEmptyDatabaseManager(); databaseStoragePath = "/a/b/some-path"; cliServer = mockedObject({}); + databaseFetcher = new DatabaseFetcher(); config = mockedObject({ download: "ask", update: "ask", @@ -74,6 +77,7 @@ describe("GitHubDatabasesModule", () => { gitHubDatabasesModule = new GitHubDatabasesModule( app, databaseManager, + databaseFetcher, databaseStoragePath, cliServer, config, @@ -124,6 +128,7 @@ describe("GitHubDatabasesModule", () => { gitHubDatabasesModule = new GitHubDatabasesModule( app, databaseManager, + databaseFetcher, databaseStoragePath, cliServer, config, @@ -207,6 +212,7 @@ describe("GitHubDatabasesModule", () => { repo, databases, databaseManager, + databaseFetcher, databaseStoragePath, cliServer, app.commands, @@ -250,6 +256,7 @@ describe("GitHubDatabasesModule", () => { repo, databaseUpdates, databaseManager, + databaseFetcher, databaseStoragePath, cliServer, app.commands, diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/updates.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/updates.test.ts index d612f95ffd1..d693e1a8968 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/updates.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/updates.test.ts @@ -12,7 +12,7 @@ import type { DatabaseManager } from "../../../../../src/databases/local-databas import type { GitHubDatabaseConfig } from "../../../../../src/config"; import type { CodeQLCliServer } from "../../../../../src/codeql-cli/cli"; import { createMockCommandManager } from "../../../../__mocks__/commandsMock"; -import * as databaseFetcher from "../../../../../src/databases/database-fetcher"; +import { DatabaseFetcher } from "../../../../../src/databases/database-fetcher"; import * as dialog from "../../../../../src/common/vscode/dialog"; import type { DatabaseUpdate } from "../../../../../src/databases/github-databases/updates"; import { @@ -344,6 +344,7 @@ describe("downloadDatabaseUpdateFromGitHub", () => { const owner = "github"; const repo = "codeql"; let databaseManager: DatabaseManager; + let databaseFetcher: DatabaseFetcher; const storagePath = "/a/b/c/d"; let cliServer: CodeQLCliServer; const commandManager = createMockCommandManager(); @@ -368,7 +369,7 @@ describe("downloadDatabaseUpdateFromGitHub", () => { let showQuickPickSpy: jest.SpiedFunction; let downloadGitHubDatabaseFromUrlSpy: jest.SpiedFunction< - typeof databaseFetcher.downloadGitHubDatabaseFromUrl + DatabaseFetcher["downloadGitHubDatabaseFromUrl"] >; beforeEach(() => { @@ -376,6 +377,7 @@ describe("downloadDatabaseUpdateFromGitHub", () => { databaseManager = mockedObject({ currentDatabaseItem: mockDatabaseItem(), }); + databaseFetcher = new DatabaseFetcher(); cliServer = mockedObject({}); showQuickPickSpy = jest.spyOn(window, "showQuickPick").mockResolvedValue( @@ -397,6 +399,7 @@ describe("downloadDatabaseUpdateFromGitHub", () => { repo, updates, databaseManager, + databaseFetcher, storagePath, cliServer, commandManager, @@ -476,6 +479,7 @@ describe("downloadDatabaseUpdateFromGitHub", () => { repo, updates, databaseManager, + databaseFetcher, storagePath, cliServer, commandManager, @@ -532,6 +536,7 @@ describe("downloadDatabaseUpdateFromGitHub", () => { repo, updates, databaseManager, + databaseFetcher, storagePath, cliServer, commandManager, @@ -597,6 +602,7 @@ describe("downloadDatabaseUpdateFromGitHub", () => { repo, updates, databaseManager, + databaseFetcher, storagePath, cliServer, commandManager, From 3ee081e8c6f05d571b3038e2d41e539d572541fd Mon Sep 17 00:00:00 2001 From: Robert Date: Thu, 7 Mar 2024 12:34:02 +0000 Subject: [PATCH 2/9] Move commonly-used field to DatabaseFetcher constructor --- .../src/databases/database-fetcher.ts | 103 ++++-------------- .../databases/github-databases/download.ts | 8 -- .../github-databases-module.ts | 12 -- .../src/databases/github-databases/updates.ts | 6 - .../src/databases/local-databases-ui.ts | 30 ++--- extensions/ql-vscode/src/extension.ts | 4 +- .../src/local-queries/local-queries.ts | 9 +- .../local-queries/skeleton-query-wizard.ts | 9 -- .../src/model-editor/model-editor-view.ts | 23 ++-- .../databases/database-fetcher.test.ts | 19 ++-- .../skeleton-query-wizard.test.ts | 16 +-- .../test/vscode-tests/global.helper.ts | 12 +- .../github-databases/download.test.ts | 43 ++------ .../github-databases-module.test.ts | 16 +-- .../github-databases/updates.test.ts | 39 ++----- 15 files changed, 115 insertions(+), 234 deletions(-) diff --git a/extensions/ql-vscode/src/databases/database-fetcher.ts b/extensions/ql-vscode/src/databases/database-fetcher.ts index 4c983e43b0f..cbaedabc6bf 100644 --- a/extensions/ql-vscode/src/databases/database-fetcher.ts +++ b/extensions/ql-vscode/src/databases/database-fetcher.ts @@ -26,7 +26,6 @@ import { getNwoFromGitHubUrl, isValidGitHubNwo, } from "../common/github-url-identifier-helper"; -import type { AppCommandManager } from "../common/commands"; import { addDatabaseSourceToWorkspace, allowHttp, @@ -48,17 +47,23 @@ const DUPLICATE_FILENAMES_TRIES = 10_000; export class DatabaseFetcher { /** - * Prompts a user to fetch a database from a remote location. Database is assumed to be an archive file. - * + * @param app the App * @param databaseManager the DatabaseManager * @param storagePath where to store the unzipped database. + * @param cli the CodeQL CLI server + **/ + constructor( + private readonly app: App, + private readonly databaseManager: DatabaseManager, + private readonly storagePath: string, + private readonly cli: CodeQLCliServer, + ) {} + + /** + * Prompts a user to fetch a database from a remote location. Database is assumed to be an archive file. */ public async promptImportInternetDatabase( - commandManager: AppCommandManager, - databaseManager: DatabaseManager, - storagePath: string, progress: ProgressCallback, - cli: CodeQLCliServer, ): Promise { const databaseUrl = await window.showInputBox({ prompt: "Enter URL of zipfile of database to download", @@ -72,19 +77,16 @@ export class DatabaseFetcher { const item = await this.databaseArchiveFetcher( databaseUrl, {}, - databaseManager, - storagePath, undefined, { type: "url", url: databaseUrl, }, progress, - cli, ); if (item) { - await commandManager.execute("codeQLDatabases.focus"); + await this.app.commands.execute("codeQLDatabases.focus"); void showAndLogInformationMessage( extLogger, "Database downloaded and imported successfully.", @@ -98,21 +100,13 @@ export class DatabaseFetcher { * User enters a GitHub repository and then the user is asked which language * to download (if there is more than one) * - * @param app the App - * @param databaseManager the DatabaseManager - * @param storagePath where to store the unzipped database. * @param progress the progress callback - * @param cli the CodeQL CLI server * @param language the language to download. If undefined, the user will be prompted to choose a language. * @param makeSelected make the new database selected in the databases panel (default: true) * @param addSourceArchiveFolder whether to add a workspace folder containing the source archive to the workspace */ public async promptImportGithubDatabase( - app: App, - databaseManager: DatabaseManager, - storagePath: string, progress: ProgressCallback, - cli: CodeQLCliServer, language?: string, makeSelected = true, addSourceArchiveFolder = addDatabaseSourceToWorkspace(), @@ -124,11 +118,7 @@ export class DatabaseFetcher { const databaseItem = await this.downloadGitHubDatabase( githubRepo, - app, - databaseManager, - storagePath, progress, - cli, language, makeSelected, addSourceArchiveFolder, @@ -136,7 +126,7 @@ export class DatabaseFetcher { if (databaseItem) { if (makeSelected) { - await app.commands.execute("codeQLDatabases.focus"); + await this.app.commands.execute("codeQLDatabases.focus"); } void showAndLogInformationMessage( extLogger, @@ -176,22 +166,14 @@ export class DatabaseFetcher { * Downloads a database from GitHub * * @param githubRepo the GitHub repository to download the database from - * @param app the App - * @param databaseManager the DatabaseManager - * @param storagePath where to store the unzipped database. * @param progress the progress callback - * @param cli the CodeQL CLI server * @param language the language to download. If undefined, the user will be prompted to choose a language. * @param makeSelected make the new database selected in the databases panel (default: true) * @param addSourceArchiveFolder whether to add a workspace folder containing the source archive to the workspace **/ public async downloadGitHubDatabase( githubRepo: string, - app: App, - databaseManager: DatabaseManager, - storagePath: string, progress: ProgressCallback, - cli: CodeQLCliServer, language?: string, makeSelected = true, addSourceArchiveFolder = addDatabaseSourceToWorkspace(), @@ -201,7 +183,7 @@ export class DatabaseFetcher { throw new Error(`Invalid GitHub repository: ${githubRepo}`); } - const credentials = isCanary() ? app.credentials : undefined; + const credentials = isCanary() ? this.app.credentials : undefined; const octokit = credentials ? await credentials.getOctokit() @@ -235,9 +217,6 @@ export class DatabaseFetcher { name, octokit, progress, - databaseManager, - storagePath, - cli, makeSelected, addSourceArchiveFolder, ); @@ -252,9 +231,6 @@ export class DatabaseFetcher { name: string, octokit: Octokit, progress: ProgressCallback, - databaseManager: DatabaseManager, - storagePath: string, - cli: CodeQLCliServer, makeSelected = true, addSourceArchiveFolder = true, ): Promise { @@ -275,8 +251,6 @@ export class DatabaseFetcher { Accept: "application/zip", Authorization: octokitToken ? `Bearer ${octokitToken}` : "", }, - databaseManager, - storagePath, `${owner}/${name}`, { type: "github", @@ -286,7 +260,6 @@ export class DatabaseFetcher { commitOid, }, progress, - cli, makeSelected, addSourceArchiveFolder, ); @@ -296,34 +269,24 @@ export class DatabaseFetcher { * Imports a database from a local archive. * * @param databaseUrl the file url of the archive to import - * @param databaseManager the DatabaseManager - * @param storagePath where to store the unzipped database. - * @param cli the CodeQL CLI server */ public async importArchiveDatabase( - commandManager: AppCommandManager, databaseUrl: string, - databaseManager: DatabaseManager, - storagePath: string, progress: ProgressCallback, - cli: CodeQLCliServer, ): Promise { try { const item = await this.databaseArchiveFetcher( databaseUrl, {}, - databaseManager, - storagePath, undefined, { type: "archive", path: databaseUrl, }, progress, - cli, ); if (item) { - await commandManager.execute("codeQLDatabases.focus"); + await this.app.commands.execute("codeQLDatabases.focus"); void showAndLogInformationMessage( extLogger, "Database unzipped and imported successfully.", @@ -348,24 +311,18 @@ export class DatabaseFetcher { * * @param databaseUrl URL from which to grab the database * @param requestHeaders Headers to send with the request - * @param databaseManager the DatabaseManager - * @param storagePath where to store the unzipped database. * @param nameOverride a name for the database that overrides the default * @param origin the origin of the database * @param progress callback to send progress messages to - * @param cli the CodeQL CLI server * @param makeSelected make the new database selected in the databases panel (default: true) * @param addSourceArchiveFolder whether to add a workspace folder containing the source archive to the workspace */ private async databaseArchiveFetcher( databaseUrl: string, requestHeaders: { [key: string]: string }, - databaseManager: DatabaseManager, - storagePath: string, nameOverride: string | undefined, origin: DatabaseOrigin, progress: ProgressCallback, - cli: CodeQLCliServer, makeSelected = true, addSourceArchiveFolder = addDatabaseSourceToWorkspace(), ): Promise { @@ -374,24 +331,19 @@ export class DatabaseFetcher { step: 1, maxStep: 4, }); - if (!storagePath) { + if (!this.storagePath) { throw new Error("No storage path specified."); } - await ensureDir(storagePath); - const unzipPath = await this.getStorageFolder( - storagePath, - databaseUrl, - nameOverride, - ); + await ensureDir(this.storagePath); + const unzipPath = await this.getStorageFolder(databaseUrl, nameOverride); if (this.isFile(databaseUrl)) { - await this.readAndUnzip(databaseUrl, unzipPath, cli, progress); + await this.readAndUnzip(databaseUrl, unzipPath, progress); } else { await this.fetchAndUnzip( databaseUrl, requestHeaders, unzipPath, - cli, progress, ); } @@ -416,7 +368,7 @@ export class DatabaseFetcher { }); await this.ensureZippedSourceLocation(dbPath); - const item = await databaseManager.openDatabase( + const item = await this.databaseManager.openDatabase( Uri.file(dbPath), origin, makeSelected, @@ -432,11 +384,7 @@ export class DatabaseFetcher { } } - private async getStorageFolder( - storagePath: string, - urlStr: string, - nameOverrride?: string, - ) { + private async getStorageFolder(urlStr: string, nameOverrride?: string) { let lastName: string; if (nameOverrride) { @@ -454,7 +402,7 @@ export class DatabaseFetcher { } } - const realpath = await fs_realpath(storagePath); + const realpath = await fs_realpath(this.storagePath); let folderName = lastName; // get all existing files instead of calling pathExists on every @@ -500,7 +448,6 @@ export class DatabaseFetcher { private async readAndUnzip( zipUrl: string, unzipPath: string, - cli: CodeQLCliServer, progress?: ProgressCallback, ) { const zipFile = Uri.parse(zipUrl).fsPath; @@ -510,14 +457,13 @@ export class DatabaseFetcher { message: `Unzipping into ${basename(unzipPath)}`, }); - await cli.databaseUnbundle(zipFile, unzipPath); + await this.cli.databaseUnbundle(zipFile, unzipPath); } private async fetchAndUnzip( databaseUrl: string, requestHeaders: { [key: string]: string }, unzipPath: string, - cli: CodeQLCliServer, progress?: ProgressCallback, ) { // Although it is possible to download and stream directly to an unzipped directory, @@ -606,7 +552,6 @@ export class DatabaseFetcher { await this.readAndUnzip( Uri.file(archivePath).toString(true), unzipPath, - cli, progress, ); diff --git a/extensions/ql-vscode/src/databases/github-databases/download.ts b/extensions/ql-vscode/src/databases/github-databases/download.ts index 89dc7527e89..e417a18505e 100644 --- a/extensions/ql-vscode/src/databases/github-databases/download.ts +++ b/extensions/ql-vscode/src/databases/github-databases/download.ts @@ -4,8 +4,6 @@ import { showNeverAskAgainDialog } from "../../common/vscode/dialog"; import { getLanguageDisplayName } from "../../common/query-language"; import type { DatabaseFetcher } from "../database-fetcher"; import { withProgress } from "../../common/vscode/progress"; -import type { DatabaseManager } from "../local-databases"; -import type { CodeQLCliServer } from "../../codeql-cli/cli"; import type { AppCommandManager } from "../../common/commands"; import type { GitHubDatabaseConfig } from "../../config"; import type { CodeqlDatabase } from "./api"; @@ -58,10 +56,7 @@ export async function downloadDatabaseFromGitHub( owner: string, repo: string, databases: CodeqlDatabase[], - databaseManager: DatabaseManager, databaseFetcher: DatabaseFetcher, - storagePath: string, - cliServer: CodeQLCliServer, commandManager: AppCommandManager, ): Promise { const selectedDatabases = await promptForDatabases(databases); @@ -82,9 +77,6 @@ export async function downloadDatabaseFromGitHub( repo, octokit, progress, - databaseManager, - storagePath, - cliServer, true, false, ); diff --git a/extensions/ql-vscode/src/databases/github-databases/github-databases-module.ts b/extensions/ql-vscode/src/databases/github-databases/github-databases-module.ts index 382d638fa42..6c97dd36b37 100644 --- a/extensions/ql-vscode/src/databases/github-databases/github-databases-module.ts +++ b/extensions/ql-vscode/src/databases/github-databases/github-databases-module.ts @@ -14,7 +14,6 @@ import { } from "./download"; import type { GitHubDatabaseConfig } from "../../config"; import type { DatabaseManager } from "../local-databases"; -import type { CodeQLCliServer } from "../../codeql-cli/cli"; import type { CodeqlDatabase, ListDatabasesResult } from "./api"; import { listDatabases } from "./api"; import type { DatabaseUpdate } from "./updates"; @@ -35,8 +34,6 @@ export class GitHubDatabasesModule extends DisposableObject { private readonly app: App, private readonly databaseManager: DatabaseManager, private readonly databaseFetcher: DatabaseFetcher, - private readonly databaseStoragePath: string, - private readonly cliServer: CodeQLCliServer, private readonly config: GitHubDatabaseConfig, ) { super(); @@ -46,16 +43,12 @@ export class GitHubDatabasesModule extends DisposableObject { app: App, databaseManager: DatabaseManager, databaseFetcher: DatabaseFetcher, - databaseStoragePath: string, - cliServer: CodeQLCliServer, config: GitHubDatabaseConfig, ): Promise { const githubDatabasesModule = new GitHubDatabasesModule( app, databaseManager, databaseFetcher, - databaseStoragePath, - cliServer, config, ); app.subscriptions.push(githubDatabasesModule); @@ -189,10 +182,7 @@ export class GitHubDatabasesModule extends DisposableObject { owner, repo, databases, - this.databaseManager, this.databaseFetcher, - this.databaseStoragePath, - this.cliServer, this.app.commands, ); } @@ -218,8 +208,6 @@ export class GitHubDatabasesModule extends DisposableObject { databaseUpdates, this.databaseManager, this.databaseFetcher, - this.databaseStoragePath, - this.cliServer, this.app.commands, ); } diff --git a/extensions/ql-vscode/src/databases/github-databases/updates.ts b/extensions/ql-vscode/src/databases/github-databases/updates.ts index 349b317ce04..5b589649fea 100644 --- a/extensions/ql-vscode/src/databases/github-databases/updates.ts +++ b/extensions/ql-vscode/src/databases/github-databases/updates.ts @@ -1,7 +1,6 @@ import type { CodeqlDatabase } from "./api"; import type { DatabaseItem, DatabaseManager } from "../local-databases"; import type { Octokit } from "@octokit/rest"; -import type { CodeQLCliServer } from "../../codeql-cli/cli"; import type { AppCommandManager } from "../../common/commands"; import { getLanguageDisplayName } from "../../common/query-language"; import { showNeverAskAgainDialog } from "../../common/vscode/dialog"; @@ -157,8 +156,6 @@ export async function downloadDatabaseUpdateFromGitHub( updates: DatabaseUpdate[], databaseManager: DatabaseManager, databaseFetcher: DatabaseFetcher, - storagePath: string, - cliServer: CodeQLCliServer, commandManager: AppCommandManager, ): Promise { const selectedDatabases = await promptForDatabases( @@ -190,9 +187,6 @@ export async function downloadDatabaseUpdateFromGitHub( repo, octokit, progress, - databaseManager, - storagePath, - cliServer, databaseManager.currentDatabaseItem === update.databaseItem, update.databaseItem.hasSourceArchiveInExplorer(), ); diff --git a/extensions/ql-vscode/src/databases/local-databases-ui.ts b/extensions/ql-vscode/src/databases/local-databases-ui.ts index 59cfb4d7206..74d33062210 100644 --- a/extensions/ql-vscode/src/databases/local-databases-ui.ts +++ b/extensions/ql-vscode/src/databases/local-databases-ui.ts @@ -536,13 +536,13 @@ export class DatabaseUI extends DisposableObject { private async handleChooseDatabaseInternet(): Promise { return withProgress( async (progress) => { - await new DatabaseFetcher().promptImportInternetDatabase( - this.app.commands, + const databaseFetcher = new DatabaseFetcher( + this.app, this.databaseManager, this.storagePath, - progress, this.queryServer.cliServer, ); + await databaseFetcher.promptImportInternetDatabase(progress); }, { title: "Adding database from URL", @@ -553,13 +553,13 @@ export class DatabaseUI extends DisposableObject { private async handleChooseDatabaseGithub(): Promise { return withProgress( async (progress) => { - await new DatabaseFetcher().promptImportGithubDatabase( + const databaseFetcher = new DatabaseFetcher( this.app, this.databaseManager, this.storagePath, - progress, this.queryServer.cliServer, ); + await databaseFetcher.promptImportGithubDatabase(progress); }, { title: "Adding database from GitHub", @@ -708,14 +708,16 @@ export class DatabaseUI extends DisposableObject { try { // Assume user has selected an archive if the file has a .zip extension if (uri.path.endsWith(".zip")) { - await new DatabaseFetcher().importArchiveDatabase( - this.app.commands, - uri.toString(true), + const databaseFetcher = new DatabaseFetcher( + this.app, this.databaseManager, this.storagePath, - progress, this.queryServer.cliServer, ); + await databaseFetcher.importArchiveDatabase( + uri.toString(true), + progress, + ); } else { await this.databaseManager.openDatabase(uri, { type: "folder", @@ -955,14 +957,16 @@ export class DatabaseUI extends DisposableObject { } else { // we are selecting a database archive. Must unzip into a workspace-controlled area // before importing. - return await new DatabaseFetcher().importArchiveDatabase( - this.app.commands, - uri.toString(true), + const databaseFetcher = new DatabaseFetcher( + this.app, this.databaseManager, this.storagePath, - progress, this.queryServer.cliServer, ); + return await databaseFetcher.importArchiveDatabase( + uri.toString(true), + progress, + ); } }, { diff --git a/extensions/ql-vscode/src/extension.ts b/extensions/ql-vscode/src/extension.ts index 5658788e582..dc2cb4bca89 100644 --- a/extensions/ql-vscode/src/extension.ts +++ b/extensions/ql-vscode/src/extension.ts @@ -882,9 +882,7 @@ async function activateWithInstalledDistribution( await GitHubDatabasesModule.initialize( app, dbm, - new DatabaseFetcher(), - getContextStoragePath(ctx), - cliServer, + new DatabaseFetcher(app, dbm, getContextStoragePath(ctx), cliServer), githubDatabaseConfigListener, ); diff --git a/extensions/ql-vscode/src/local-queries/local-queries.ts b/extensions/ql-vscode/src/local-queries/local-queries.ts index ea1c5f8e864..9d47f8f909d 100644 --- a/extensions/ql-vscode/src/local-queries/local-queries.ts +++ b/extensions/ql-vscode/src/local-queries/local-queries.ts @@ -326,13 +326,18 @@ export class LocalQueries extends DisposableObject { const contextStoragePath = this.app.workspaceStoragePath || this.app.globalStoragePath; const language = this.languageContextStore.selectedLanguage; + const databaseFetcher = new DatabaseFetcher( + this.app, + this.databaseManager, + contextStoragePath, + this.cliServer, + ); const skeletonQueryWizard = new SkeletonQueryWizard( this.cliServer, progress, this.app, this.databaseManager, - new DatabaseFetcher(), - contextStoragePath, + databaseFetcher, this.selectedQueryTreeViewItems, language, ); diff --git a/extensions/ql-vscode/src/local-queries/skeleton-query-wizard.ts b/extensions/ql-vscode/src/local-queries/skeleton-query-wizard.ts index 5305fe40a01..fb96dd666da 100644 --- a/extensions/ql-vscode/src/local-queries/skeleton-query-wizard.ts +++ b/extensions/ql-vscode/src/local-queries/skeleton-query-wizard.ts @@ -60,7 +60,6 @@ export class SkeletonQueryWizard { private readonly app: App, private readonly databaseManager: DatabaseManager, private readonly databaseFetcher: DatabaseFetcher, - private readonly databaseStoragePath: string | undefined, private readonly selectedItems: readonly QueryTreeViewItem[], private language: QueryLanguage | undefined = undefined, ) {} @@ -361,10 +360,6 @@ export class SkeletonQueryWizard { } private async downloadDatabase(progress: ProgressCallback) { - if (this.databaseStoragePath === undefined) { - throw new Error("Database storage path is undefined"); - } - if (this.language === undefined) { throw new Error("Language is undefined"); } @@ -387,11 +382,7 @@ export class SkeletonQueryWizard { await this.databaseFetcher.downloadGitHubDatabase( chosenRepo, - this.app, - this.databaseManager, - this.databaseStoragePath, progress, - this.cliServer, this.language, ); } diff --git a/extensions/ql-vscode/src/model-editor/model-editor-view.ts b/extensions/ql-vscode/src/model-editor/model-editor-view.ts index 2e72981acd7..feb2581f75c 100644 --- a/extensions/ql-vscode/src/model-editor/model-editor-view.ts +++ b/extensions/ql-vscode/src/model-editor/model-editor-view.ts @@ -916,17 +916,18 @@ export class ModelEditorView extends AbstractWebview< // the user to import the library database. We need to have the database // imported to the query server, so we need to register it to our workspace. const makeSelected = false; - const addedDatabase = - await new DatabaseFetcher().promptImportGithubDatabase( - this.app, - this.databaseManager, - this.app.workspaceStoragePath ?? this.app.globalStoragePath, - progress, - this.cliServer, - this.databaseItem.language, - makeSelected, - false, - ); + const databaseFetcher = new DatabaseFetcher( + this.app, + this.databaseManager, + this.app.workspaceStoragePath ?? this.app.globalStoragePath, + this.cliServer, + ); + const addedDatabase = await databaseFetcher.promptImportGithubDatabase( + progress, + this.databaseItem.language, + makeSelected, + false, + ); if (!addedDatabase) { void this.app.logger.log("No database chosen"); return; diff --git a/extensions/ql-vscode/test/vscode-tests/cli-integration/databases/database-fetcher.test.ts b/extensions/ql-vscode/test/vscode-tests/cli-integration/databases/database-fetcher.test.ts index cc4b6f2d5a4..ff28dced4c5 100644 --- a/extensions/ql-vscode/test/vscode-tests/cli-integration/databases/database-fetcher.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/cli-integration/databases/database-fetcher.test.ts @@ -11,8 +11,8 @@ import { getActivatedExtension, storagePath, } from "../../global.helper"; -import { createMockCommandManager } from "../../../__mocks__/commandsMock"; import { remove } from "fs-extra"; +import { createMockApp } from "../../../__mocks__/appMock"; /** * Run various integration tests for databases @@ -46,14 +46,16 @@ describe("database-fetcher", () => { describe("importArchiveDatabase", () => { it("should add a database from a folder", async () => { const uri = Uri.file(dbLoc); - let dbItem = await new DatabaseFetcher().importArchiveDatabase( - createMockCommandManager(), - uri.toString(true), + const databaseFetcher = new DatabaseFetcher( + createMockApp(), databaseManager, storagePath, - progressCallback, cli, ); + let dbItem = await databaseFetcher.importArchiveDatabase( + uri.toString(true), + progressCallback, + ); expect(dbItem).toBe(databaseManager.currentDatabaseItem); expect(dbItem).toBe(databaseManager.databaseItems[0]); expect(dbItem).toBeDefined(); @@ -68,13 +70,14 @@ describe("database-fetcher", () => { // Provide a database URL when prompted inputBoxStub.mockResolvedValue(DB_URL); - let dbItem = await new DatabaseFetcher().promptImportInternetDatabase( - createMockCommandManager(), + const databaseFetcher = new DatabaseFetcher( + createMockApp(), databaseManager, storagePath, - progressCallback, cli, ); + let dbItem = + await databaseFetcher.promptImportInternetDatabase(progressCallback); expect(dbItem).toBeDefined(); dbItem = dbItem!; expect(dbItem.name).toBe("db"); diff --git a/extensions/ql-vscode/test/vscode-tests/cli-integration/local-queries/skeleton-query-wizard.test.ts b/extensions/ql-vscode/test/vscode-tests/cli-integration/local-queries/skeleton-query-wizard.test.ts index 319a0e8cfe9..b2542a48b75 100644 --- a/extensions/ql-vscode/test/vscode-tests/cli-integration/local-queries/skeleton-query-wizard.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/cli-integration/local-queries/skeleton-query-wizard.test.ts @@ -116,7 +116,12 @@ describe("SkeletonQueryWizard", () => { }, ] as WorkspaceFolder[]); - databaseFetcher = new DatabaseFetcher(); + databaseFetcher = new DatabaseFetcher( + mockApp, + mockDatabaseManager, + storagePath, + mockCli, + ); quickPickSpy = jest.spyOn(window, "showQuickPick").mockResolvedValueOnce( mockedQuickPickItem({ @@ -149,7 +154,6 @@ describe("SkeletonQueryWizard", () => { mockApp, mockDatabaseManager, databaseFetcher, - storagePath, selectedItems, ); @@ -177,7 +181,6 @@ describe("SkeletonQueryWizard", () => { mockApp, mockDatabaseManager, databaseFetcher, - storagePath, selectedItems, QueryLanguage.Swift, ); @@ -326,7 +329,6 @@ describe("SkeletonQueryWizard", () => { mockApp, mockDatabaseManagerWithItems, databaseFetcher, - storagePath, selectedItems, ); }); @@ -376,7 +378,6 @@ describe("SkeletonQueryWizard", () => { mockApp, mockDatabaseManagerWithItems, databaseFetcher, - storagePath, selectedItems, ); }); @@ -512,7 +513,6 @@ describe("SkeletonQueryWizard", () => { mockApp, mockDatabaseManager, databaseFetcher, - storagePath, selectedItems, QueryLanguage.Javascript, ); @@ -734,7 +734,6 @@ describe("SkeletonQueryWizard", () => { mockApp, mockDatabaseManager, databaseFetcher, - storagePath, selectedItems, ); }); @@ -764,7 +763,6 @@ describe("SkeletonQueryWizard", () => { mockApp, mockDatabaseManager, databaseFetcher, - storagePath, selectedItems, ); }); @@ -798,7 +796,6 @@ describe("SkeletonQueryWizard", () => { mockApp, mockDatabaseManager, databaseFetcher, - storagePath, selectedItems, QueryLanguage.Swift, ); @@ -842,7 +839,6 @@ describe("SkeletonQueryWizard", () => { mockApp, mockDatabaseManager, databaseFetcher, - storagePath, selectedItems, ); }); diff --git a/extensions/ql-vscode/test/vscode-tests/global.helper.ts b/extensions/ql-vscode/test/vscode-tests/global.helper.ts index b0f85a8ec09..1660f3d57af 100644 --- a/extensions/ql-vscode/test/vscode-tests/global.helper.ts +++ b/extensions/ql-vscode/test/vscode-tests/global.helper.ts @@ -8,7 +8,7 @@ import type { import type { CodeQLCliServer } from "../../src/codeql-cli/cli"; import type { CodeQLExtensionInterface } from "../../src/extension"; import { DatabaseFetcher } from "../../src/databases/database-fetcher"; -import { createMockCommandManager } from "../__mocks__/commandsMock"; +import { createMockApp } from "../__mocks__/appMock"; // This file contains helpers shared between tests that work with an activated extension. @@ -34,15 +34,17 @@ export async function ensureTestDatabase( // Add a database, but make sure the database manager is empty first await cleanDatabases(databaseManager); const uri = Uri.file(dbLoc); - const maybeDbItem = await new DatabaseFetcher().importArchiveDatabase( - createMockCommandManager(), - uri.toString(true), + const databaseFetcher = new DatabaseFetcher( + createMockApp(), databaseManager, storagePath, + cli, + ); + const maybeDbItem = await databaseFetcher.importArchiveDatabase( + uri.toString(true), (_p) => { /**/ }, - cli, ); if (!maybeDbItem) { diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/download.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/download.test.ts index 3768dca4992..b38c974cc24 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/download.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/download.test.ts @@ -13,10 +13,10 @@ import { import type { DatabaseManager } from "../../../../../src/databases/local-databases"; import type { GitHubDatabaseConfig } from "../../../../../src/config"; import type { CodeQLCliServer } from "../../../../../src/codeql-cli/cli"; -import { createMockCommandManager } from "../../../../__mocks__/commandsMock"; import { DatabaseFetcher } from "../../../../../src/databases/database-fetcher"; import * as dialog from "../../../../../src/common/vscode/dialog"; import type { CodeqlDatabase } from "../../../../../src/databases/github-databases/api"; +import { createMockApp } from "../../../../__mocks__/appMock"; describe("askForGitHubDatabaseDownload", () => { const setDownload = jest.fn(); @@ -101,7 +101,7 @@ describe("downloadDatabaseFromGitHub", () => { const storagePath = "/a/b/c/d"; let cliServer: CodeQLCliServer; - const commandManager = createMockCommandManager(); + const app = createMockApp(); let databases = [ mockedObject({ @@ -124,8 +124,13 @@ describe("downloadDatabaseFromGitHub", () => { beforeEach(() => { octokit = mockedObject({}); databaseManager = mockedObject({}); - databaseFetcher = new DatabaseFetcher(); cliServer = mockedObject({}); + databaseFetcher = new DatabaseFetcher( + app, + databaseManager, + storagePath, + cliServer, + ); showQuickPickSpy = jest.spyOn(window, "showQuickPick").mockResolvedValue( mockedQuickPickItem([ @@ -145,11 +150,8 @@ describe("downloadDatabaseFromGitHub", () => { owner, repo, databases, - databaseManager, databaseFetcher, - storagePath, - cliServer, - commandManager, + app.commands, ); expect(downloadGitHubDatabaseFromUrlSpy).toHaveBeenCalledTimes(1); @@ -162,9 +164,6 @@ describe("downloadDatabaseFromGitHub", () => { repo, octokit, expect.anything(), - databaseManager, - storagePath, - cliServer, true, false, ); @@ -210,11 +209,8 @@ describe("downloadDatabaseFromGitHub", () => { owner, repo, databases, - databaseManager, databaseFetcher, - storagePath, - cliServer, - commandManager, + app.commands, ); expect(downloadGitHubDatabaseFromUrlSpy).toHaveBeenCalledTimes(1); @@ -227,9 +223,6 @@ describe("downloadDatabaseFromGitHub", () => { repo, octokit, expect.anything(), - databaseManager, - storagePath, - cliServer, true, false, ); @@ -267,11 +260,8 @@ describe("downloadDatabaseFromGitHub", () => { owner, repo, databases, - databaseManager, databaseFetcher, - storagePath, - cliServer, - commandManager, + app.commands, ); expect(downloadGitHubDatabaseFromUrlSpy).toHaveBeenCalledTimes(2); @@ -284,9 +274,6 @@ describe("downloadDatabaseFromGitHub", () => { repo, octokit, expect.anything(), - databaseManager, - storagePath, - cliServer, true, false, ); @@ -299,9 +286,6 @@ describe("downloadDatabaseFromGitHub", () => { repo, octokit, expect.anything(), - databaseManager, - storagePath, - cliServer, true, false, ); @@ -333,11 +317,8 @@ describe("downloadDatabaseFromGitHub", () => { owner, repo, databases, - databaseManager, databaseFetcher, - storagePath, - cliServer, - commandManager, + app.commands, ); expect(downloadGitHubDatabaseFromUrlSpy).not.toHaveBeenCalled(); diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/github-databases-module.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/github-databases-module.test.ts index d5081a1c4d5..18607aab28e 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/github-databases-module.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/github-databases-module.test.ts @@ -68,7 +68,12 @@ describe("GitHubDatabasesModule", () => { databaseManager = mockEmptyDatabaseManager(); databaseStoragePath = "/a/b/some-path"; cliServer = mockedObject({}); - databaseFetcher = new DatabaseFetcher(); + databaseFetcher = new DatabaseFetcher( + app, + databaseManager, + databaseStoragePath, + cliServer, + ); config = mockedObject({ download: "ask", update: "ask", @@ -78,8 +83,6 @@ describe("GitHubDatabasesModule", () => { app, databaseManager, databaseFetcher, - databaseStoragePath, - cliServer, config, ); @@ -129,8 +132,6 @@ describe("GitHubDatabasesModule", () => { app, databaseManager, databaseFetcher, - databaseStoragePath, - cliServer, config, ); @@ -211,10 +212,7 @@ describe("GitHubDatabasesModule", () => { owner, repo, databases, - databaseManager, databaseFetcher, - databaseStoragePath, - cliServer, app.commands, ); }); @@ -257,8 +255,6 @@ describe("GitHubDatabasesModule", () => { databaseUpdates, databaseManager, databaseFetcher, - databaseStoragePath, - cliServer, app.commands, ); }); diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/updates.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/updates.test.ts index d693e1a8968..d5b61957000 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/updates.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/updates.test.ts @@ -11,7 +11,6 @@ import type { CodeqlDatabase } from "../../../../../src/databases/github-databas import type { DatabaseManager } from "../../../../../src/databases/local-databases"; import type { GitHubDatabaseConfig } from "../../../../../src/config"; import type { CodeQLCliServer } from "../../../../../src/codeql-cli/cli"; -import { createMockCommandManager } from "../../../../__mocks__/commandsMock"; import { DatabaseFetcher } from "../../../../../src/databases/database-fetcher"; import * as dialog from "../../../../../src/common/vscode/dialog"; import type { DatabaseUpdate } from "../../../../../src/databases/github-databases/updates"; @@ -20,6 +19,7 @@ import { downloadDatabaseUpdateFromGitHub, isNewerDatabaseAvailable, } from "../../../../../src/databases/github-databases/updates"; +import { createMockApp } from "../../../../__mocks__/appMock"; describe("isNewerDatabaseAvailable", () => { const owner = "github"; @@ -347,7 +347,7 @@ describe("downloadDatabaseUpdateFromGitHub", () => { let databaseFetcher: DatabaseFetcher; const storagePath = "/a/b/c/d"; let cliServer: CodeQLCliServer; - const commandManager = createMockCommandManager(); + const app = createMockApp(); let updates: DatabaseUpdate[] = [ { @@ -377,8 +377,13 @@ describe("downloadDatabaseUpdateFromGitHub", () => { databaseManager = mockedObject({ currentDatabaseItem: mockDatabaseItem(), }); - databaseFetcher = new DatabaseFetcher(); cliServer = mockedObject({}); + databaseFetcher = new DatabaseFetcher( + app, + databaseManager, + storagePath, + cliServer, + ); showQuickPickSpy = jest.spyOn(window, "showQuickPick").mockResolvedValue( mockedQuickPickItem([ @@ -400,9 +405,7 @@ describe("downloadDatabaseUpdateFromGitHub", () => { updates, databaseManager, databaseFetcher, - storagePath, - cliServer, - commandManager, + app.commands, ); expect(downloadGitHubDatabaseFromUrlSpy).toHaveBeenCalledTimes(1); @@ -415,9 +418,6 @@ describe("downloadDatabaseUpdateFromGitHub", () => { repo, octokit, expect.anything(), - databaseManager, - storagePath, - cliServer, false, false, ); @@ -480,9 +480,7 @@ describe("downloadDatabaseUpdateFromGitHub", () => { updates, databaseManager, databaseFetcher, - storagePath, - cliServer, - commandManager, + app.commands, ); expect(downloadGitHubDatabaseFromUrlSpy).toHaveBeenCalledTimes(1); @@ -495,9 +493,6 @@ describe("downloadDatabaseUpdateFromGitHub", () => { repo, octokit, expect.anything(), - databaseManager, - storagePath, - cliServer, true, true, ); @@ -537,9 +532,7 @@ describe("downloadDatabaseUpdateFromGitHub", () => { updates, databaseManager, databaseFetcher, - storagePath, - cliServer, - commandManager, + app.commands, ); expect(downloadGitHubDatabaseFromUrlSpy).toHaveBeenCalledTimes(2); @@ -552,9 +545,6 @@ describe("downloadDatabaseUpdateFromGitHub", () => { repo, octokit, expect.anything(), - databaseManager, - storagePath, - cliServer, false, false, ); @@ -567,9 +557,6 @@ describe("downloadDatabaseUpdateFromGitHub", () => { repo, octokit, expect.anything(), - databaseManager, - storagePath, - cliServer, true, true, ); @@ -603,9 +590,7 @@ describe("downloadDatabaseUpdateFromGitHub", () => { updates, databaseManager, databaseFetcher, - storagePath, - cliServer, - commandManager, + app.commands, ); expect(downloadGitHubDatabaseFromUrlSpy).not.toHaveBeenCalled(); From f29aff6303f4917c6ac01fecb45ad27b20b8cf3d Mon Sep 17 00:00:00 2001 From: Robert Date: Thu, 7 Mar 2024 12:39:57 +0000 Subject: [PATCH 3/9] Inline isFile --- extensions/ql-vscode/src/databases/database-fetcher.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/extensions/ql-vscode/src/databases/database-fetcher.ts b/extensions/ql-vscode/src/databases/database-fetcher.ts index cbaedabc6bf..49b057b8fa5 100644 --- a/extensions/ql-vscode/src/databases/database-fetcher.ts +++ b/extensions/ql-vscode/src/databases/database-fetcher.ts @@ -337,7 +337,7 @@ export class DatabaseFetcher { await ensureDir(this.storagePath); const unzipPath = await this.getStorageFolder(databaseUrl, nameOverride); - if (this.isFile(databaseUrl)) { + if (Uri.parse(databaseUrl).scheme === "file") { await this.readAndUnzip(databaseUrl, unzipPath, progress); } else { await this.fetchAndUnzip( @@ -580,10 +580,6 @@ export class DatabaseFetcher { throw new Error(`${errorMessage}.\n\nReason: ${msg}`); } - private isFile(databaseUrl: string) { - return Uri.parse(databaseUrl).scheme === "file"; - } - /** * Databases created by the old odasa tool will not have a zipped * source location. However, this extension works better if sources From 9fa6d99f09414c6eb9be55b067c77fae9701b89c Mon Sep 17 00:00:00 2001 From: Robert Date: Thu, 7 Mar 2024 12:58:21 +0000 Subject: [PATCH 4/9] Have the skeleton query wizard call promptImportGithubDatabase so we can make more methods private --- .../src/databases/database-fetcher.ts | 7 +- .../local-queries/skeleton-query-wizard.ts | 13 +-- .../src/model-editor/model-editor-view.ts | 1 + .../skeleton-query-wizard.test.ts | 82 +++++-------------- 4 files changed, 29 insertions(+), 74 deletions(-) diff --git a/extensions/ql-vscode/src/databases/database-fetcher.ts b/extensions/ql-vscode/src/databases/database-fetcher.ts index 49b057b8fa5..7b1f6aeb3f4 100644 --- a/extensions/ql-vscode/src/databases/database-fetcher.ts +++ b/extensions/ql-vscode/src/databases/database-fetcher.ts @@ -108,10 +108,11 @@ export class DatabaseFetcher { public async promptImportGithubDatabase( progress: ProgressCallback, language?: string, + suggestedRepoNwo?: string, makeSelected = true, addSourceArchiveFolder = addDatabaseSourceToWorkspace(), ): Promise { - const githubRepo = await this.askForGitHubRepo(progress); + const githubRepo = await this.askForGitHubRepo(progress, suggestedRepoNwo); if (!githubRepo) { return; } @@ -138,7 +139,7 @@ export class DatabaseFetcher { return; } - public async askForGitHubRepo( + private async askForGitHubRepo( progress?: ProgressCallback, suggestedValue?: string, ): Promise { @@ -171,7 +172,7 @@ export class DatabaseFetcher { * @param makeSelected make the new database selected in the databases panel (default: true) * @param addSourceArchiveFolder whether to add a workspace folder containing the source archive to the workspace **/ - public async downloadGitHubDatabase( + private async downloadGitHubDatabase( githubRepo: string, progress: ProgressCallback, language?: string, diff --git a/extensions/ql-vscode/src/local-queries/skeleton-query-wizard.ts b/extensions/ql-vscode/src/local-queries/skeleton-query-wizard.ts index fb96dd666da..8d9390eab96 100644 --- a/extensions/ql-vscode/src/local-queries/skeleton-query-wizard.ts +++ b/extensions/ql-vscode/src/local-queries/skeleton-query-wizard.ts @@ -371,19 +371,10 @@ export class SkeletonQueryWizard { }); const githubRepoNwo = QUERY_LANGUAGE_TO_DATABASE_REPO[this.language]; - const chosenRepo = await this.databaseFetcher.askForGitHubRepo( - undefined, - githubRepoNwo, - ); - - if (!chosenRepo) { - throw new UserCancellationException("No GitHub repository provided"); - } - - await this.databaseFetcher.downloadGitHubDatabase( - chosenRepo, + await this.databaseFetcher.promptImportGithubDatabase( progress, this.language, + githubRepoNwo, ); } diff --git a/extensions/ql-vscode/src/model-editor/model-editor-view.ts b/extensions/ql-vscode/src/model-editor/model-editor-view.ts index feb2581f75c..1d32867572b 100644 --- a/extensions/ql-vscode/src/model-editor/model-editor-view.ts +++ b/extensions/ql-vscode/src/model-editor/model-editor-view.ts @@ -925,6 +925,7 @@ export class ModelEditorView extends AbstractWebview< const addedDatabase = await databaseFetcher.promptImportGithubDatabase( progress, this.databaseItem.language, + undefined, makeSelected, false, ); diff --git a/extensions/ql-vscode/test/vscode-tests/cli-integration/local-queries/skeleton-query-wizard.test.ts b/extensions/ql-vscode/test/vscode-tests/cli-integration/local-queries/skeleton-query-wizard.test.ts index b2542a48b75..ece55e4b07c 100644 --- a/extensions/ql-vscode/test/vscode-tests/cli-integration/local-queries/skeleton-query-wizard.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/cli-integration/local-queries/skeleton-query-wizard.test.ts @@ -56,11 +56,8 @@ describe("SkeletonQueryWizard", () => { let createExampleQlFileSpy: jest.SpiedFunction< typeof QlPackGenerator.prototype.createExampleQlFile >; - let downloadGitHubDatabaseSpy: jest.SpiedFunction< - DatabaseFetcher["downloadGitHubDatabase"] - >; - let askForGitHubRepoSpy: jest.SpiedFunction< - DatabaseFetcher["askForGitHubRepo"] + let promptImportGithubDatabaseSpy: jest.SpiedFunction< + DatabaseFetcher["promptImportGithubDatabase"] >; let openTextDocumentSpy: jest.SpiedFunction< typeof workspace.openTextDocument @@ -141,8 +138,8 @@ describe("SkeletonQueryWizard", () => { createExampleQlFileSpy = jest .spyOn(QlPackGenerator.prototype, "createExampleQlFile") .mockResolvedValue(undefined); - downloadGitHubDatabaseSpy = jest - .spyOn(databaseFetcher, "downloadGitHubDatabase") + promptImportGithubDatabaseSpy = jest + .spyOn(databaseFetcher, "promptImportGithubDatabase") .mockResolvedValue(undefined); openTextDocumentSpy = jest .spyOn(workspace, "openTextDocument") @@ -156,10 +153,6 @@ describe("SkeletonQueryWizard", () => { databaseFetcher, selectedItems, ); - - askForGitHubRepoSpy = jest - .spyOn(databaseFetcher, "askForGitHubRepo") - .mockResolvedValue(QUERY_LANGUAGE_TO_DATABASE_REPO[chosenLanguage]); }); afterEach(async () => { @@ -210,7 +203,7 @@ describe("SkeletonQueryWizard", () => { title: "Download database", }), ); - expect(downloadGitHubDatabaseSpy).not.toHaveBeenCalled(); + expect(promptImportGithubDatabaseSpy).not.toHaveBeenCalled(); }); it("should download database for selected language when selecting download in prompt", async () => { @@ -227,7 +220,7 @@ describe("SkeletonQueryWizard", () => { await wizard.execute(); await wizard.waitForDownload(); - expect(downloadGitHubDatabaseSpy).toHaveBeenCalled(); + expect(promptImportGithubDatabaseSpy).toHaveBeenCalled(); }); it("should open the query file", async () => { @@ -336,7 +329,7 @@ describe("SkeletonQueryWizard", () => { it("should not download a new database for language", async () => { await wizard.execute(); - expect(downloadGitHubDatabaseSpy).not.toHaveBeenCalled(); + expect(promptImportGithubDatabaseSpy).not.toHaveBeenCalled(); }); it("should not select the database", async () => { @@ -385,7 +378,7 @@ describe("SkeletonQueryWizard", () => { it("should not download a new database for language", async () => { await wizard.execute(); - expect(downloadGitHubDatabaseSpy).not.toHaveBeenCalled(); + expect(promptImportGithubDatabaseSpy).not.toHaveBeenCalled(); }); it("should select an existing database", async () => { @@ -417,54 +410,23 @@ describe("SkeletonQueryWizard", () => { }); describe("if database is missing", () => { - describe("if the user chooses to downloaded the suggested database from GitHub", () => { - beforeEach(() => { - showInformationMessageSpy.mockImplementation( - async (_message, options, item) => { - if (item === undefined) { - return options as MessageItem; - } - - return item; - }, - ); - }); - - it("should download a new database for language", async () => { - await wizard.execute(); - await wizard.waitForDownload(); - - expect(askForGitHubRepoSpy).toHaveBeenCalled(); - expect(downloadGitHubDatabaseSpy).toHaveBeenCalled(); - }); + beforeEach(() => { + showInformationMessageSpy.mockImplementation( + async (_message, options, item) => { + if (item === undefined) { + return options as MessageItem; + } + + return item; + }, + ); }); - describe("if the user choses to download a different database from GitHub than the one suggested", () => { - beforeEach(() => { - showInformationMessageSpy.mockImplementation( - async (_message, options, item) => { - if (item === undefined) { - return options as MessageItem; - } - - return item; - }, - ); - - const chosenGitHubRepo = "pickles-owner/pickles-repo"; - - askForGitHubRepoSpy = jest - .spyOn(databaseFetcher, "askForGitHubRepo") - .mockResolvedValue(chosenGitHubRepo); - }); - - it("should download the newly chosen database", async () => { - await wizard.execute(); - await wizard.waitForDownload(); + it("should download a new database for language", async () => { + await wizard.execute(); + await wizard.waitForDownload(); - expect(askForGitHubRepoSpy).toHaveBeenCalled(); - expect(downloadGitHubDatabaseSpy).toHaveBeenCalled(); - }); + expect(promptImportGithubDatabaseSpy).toHaveBeenCalled(); }); }); }); From ec427283678d5e659cf11b66fb6e37254ed6dab7 Mon Sep 17 00:00:00 2001 From: Robert Date: Thu, 7 Mar 2024 14:58:45 +0000 Subject: [PATCH 5/9] Only construct DatabaseFetcher once in extension.ts --- .../src/databases/local-databases-ui.ts | 35 ++++--------------- extensions/ql-vscode/src/extension.ts | 12 ++++++- .../src/local-queries/local-queries.ts | 13 ++----- .../src/model-editor/model-editor-module.ts | 5 +++ .../src/model-editor/model-editor-view.ts | 12 +++---- .../github-databases-module.test.ts | 15 ++------ .../databases/local-databases-ui.test.ts | 4 +++ .../model-editor/model-editor-view.test.ts | 3 ++ 8 files changed, 38 insertions(+), 61 deletions(-) diff --git a/extensions/ql-vscode/src/databases/local-databases-ui.ts b/extensions/ql-vscode/src/databases/local-databases-ui.ts index 74d33062210..d4cc9d16537 100644 --- a/extensions/ql-vscode/src/databases/local-databases-ui.ts +++ b/extensions/ql-vscode/src/databases/local-databases-ui.ts @@ -42,7 +42,7 @@ import { showAndLogExceptionWithTelemetry, showAndLogErrorMessage, } from "../common/logging"; -import { DatabaseFetcher } from "./database-fetcher"; +import type { DatabaseFetcher } from "./database-fetcher"; import { asError, asyncFilter, getErrorMessage } from "../common/helpers-pure"; import type { QueryRunner } from "../query-server"; import type { App } from "../common/app"; @@ -248,6 +248,7 @@ export class DatabaseUI extends DisposableObject { public constructor( private app: App, private databaseManager: DatabaseManager, + private readonly databaseFetcher: DatabaseFetcher, languageContext: LanguageContextStore, private readonly queryServer: QueryRunner, private readonly storagePath: string, @@ -536,13 +537,7 @@ export class DatabaseUI extends DisposableObject { private async handleChooseDatabaseInternet(): Promise { return withProgress( async (progress) => { - const databaseFetcher = new DatabaseFetcher( - this.app, - this.databaseManager, - this.storagePath, - this.queryServer.cliServer, - ); - await databaseFetcher.promptImportInternetDatabase(progress); + await this.databaseFetcher.promptImportInternetDatabase(progress); }, { title: "Adding database from URL", @@ -553,13 +548,7 @@ export class DatabaseUI extends DisposableObject { private async handleChooseDatabaseGithub(): Promise { return withProgress( async (progress) => { - const databaseFetcher = new DatabaseFetcher( - this.app, - this.databaseManager, - this.storagePath, - this.queryServer.cliServer, - ); - await databaseFetcher.promptImportGithubDatabase(progress); + await this.databaseFetcher.promptImportGithubDatabase(progress); }, { title: "Adding database from GitHub", @@ -708,13 +697,7 @@ export class DatabaseUI extends DisposableObject { try { // Assume user has selected an archive if the file has a .zip extension if (uri.path.endsWith(".zip")) { - const databaseFetcher = new DatabaseFetcher( - this.app, - this.databaseManager, - this.storagePath, - this.queryServer.cliServer, - ); - await databaseFetcher.importArchiveDatabase( + await this.databaseFetcher.importArchiveDatabase( uri.toString(true), progress, ); @@ -957,13 +940,7 @@ export class DatabaseUI extends DisposableObject { } else { // we are selecting a database archive. Must unzip into a workspace-controlled area // before importing. - const databaseFetcher = new DatabaseFetcher( - this.app, - this.databaseManager, - this.storagePath, - this.queryServer.cliServer, - ); - return await databaseFetcher.importArchiveDatabase( + return await this.databaseFetcher.importArchiveDatabase( uri.toString(true), progress, ); diff --git a/extensions/ql-vscode/src/extension.ts b/extensions/ql-vscode/src/extension.ts index dc2cb4bca89..f2f61d90920 100644 --- a/extensions/ql-vscode/src/extension.ts +++ b/extensions/ql-vscode/src/extension.ts @@ -800,12 +800,20 @@ async function activateWithInstalledDistribution( // Let this run async. void dbm.loadPersistedState(); + const databaseFetcher = new DatabaseFetcher( + app, + dbm, + getContextStoragePath(ctx), + cliServer, + ); + ctx.subscriptions.push(dbm); void extLogger.log("Initializing database panel."); const databaseUI = new DatabaseUI( app, dbm, + databaseFetcher, languageContext, qs, getContextStoragePath(ctx), @@ -882,7 +890,7 @@ async function activateWithInstalledDistribution( await GitHubDatabasesModule.initialize( app, dbm, - new DatabaseFetcher(app, dbm, getContextStoragePath(ctx), cliServer), + databaseFetcher, githubDatabaseConfigListener, ); @@ -953,6 +961,7 @@ async function activateWithInstalledDistribution( qs, qhm, dbm, + databaseFetcher, cliServer, databaseUI, localQueryResultsView, @@ -977,6 +986,7 @@ async function activateWithInstalledDistribution( const modelEditorModule = await ModelEditorModule.initialize( app, dbm, + databaseFetcher, variantAnalysisManager, cliServer, qs, diff --git a/extensions/ql-vscode/src/local-queries/local-queries.ts b/extensions/ql-vscode/src/local-queries/local-queries.ts index 9d47f8f909d..d2f89eb1699 100644 --- a/extensions/ql-vscode/src/local-queries/local-queries.ts +++ b/extensions/ql-vscode/src/local-queries/local-queries.ts @@ -51,7 +51,7 @@ import type { QueryTreeViewItem } from "../queries-panel/query-tree-view-item"; import { tryGetQueryLanguage } from "../common/query-language"; import type { LanguageContextStore } from "../language-context-store"; import type { ExtensionApp } from "../common/vscode/vscode-app"; -import { DatabaseFetcher } from "../databases/database-fetcher"; +import type { DatabaseFetcher } from "../databases/database-fetcher"; export enum QuickEvalType { None, @@ -67,6 +67,7 @@ export class LocalQueries extends DisposableObject { private readonly queryRunner: QueryRunner, private readonly queryHistoryManager: QueryHistoryManager, private readonly databaseManager: DatabaseManager, + private readonly databaseFetcher: DatabaseFetcher, private readonly cliServer: CodeQLCliServer, private readonly databaseUI: DatabaseUI, private readonly localQueryResultsView: ResultsView, @@ -323,21 +324,13 @@ export class LocalQueries extends DisposableObject { private async createSkeletonQuery(): Promise { await withProgress( async (progress: ProgressCallback) => { - const contextStoragePath = - this.app.workspaceStoragePath || this.app.globalStoragePath; const language = this.languageContextStore.selectedLanguage; - const databaseFetcher = new DatabaseFetcher( - this.app, - this.databaseManager, - contextStoragePath, - this.cliServer, - ); const skeletonQueryWizard = new SkeletonQueryWizard( this.cliServer, progress, this.app, this.databaseManager, - databaseFetcher, + this.databaseFetcher, this.selectedQueryTreeViewItems, language, ); diff --git a/extensions/ql-vscode/src/model-editor/model-editor-module.ts b/extensions/ql-vscode/src/model-editor/model-editor-module.ts index 83edc7b0ed3..2aa914e377f 100644 --- a/extensions/ql-vscode/src/model-editor/model-editor-module.ts +++ b/extensions/ql-vscode/src/model-editor/model-editor-module.ts @@ -32,6 +32,7 @@ import { INITIAL_MODE } from "./shared/mode"; import { isSupportedLanguage } from "./supported-languages"; import { DefaultNotifier, checkConsistency } from "./consistency-check"; import type { VariantAnalysisManager } from "../variant-analysis/variant-analysis-manager"; +import type { DatabaseFetcher } from "../databases/database-fetcher"; export class ModelEditorModule extends DisposableObject { private readonly queryStorageDir: string; @@ -42,6 +43,7 @@ export class ModelEditorModule extends DisposableObject { private constructor( private readonly app: App, private readonly databaseManager: DatabaseManager, + private readonly databaseFetcher: DatabaseFetcher, private readonly variantAnalysisManager: VariantAnalysisManager, private readonly cliServer: CodeQLCliServer, private readonly queryRunner: QueryRunner, @@ -65,6 +67,7 @@ export class ModelEditorModule extends DisposableObject { public static async initialize( app: App, databaseManager: DatabaseManager, + databaseFetcher: DatabaseFetcher, variantAnalysisManager: VariantAnalysisManager, cliServer: CodeQLCliServer, queryRunner: QueryRunner, @@ -73,6 +76,7 @@ export class ModelEditorModule extends DisposableObject { const modelEditorModule = new ModelEditorModule( app, databaseManager, + databaseFetcher, variantAnalysisManager, cliServer, queryRunner, @@ -236,6 +240,7 @@ export class ModelEditorModule extends DisposableObject { this.modelingEvents, this.modelConfig, this.databaseManager, + this.databaseFetcher, this.variantAnalysisManager, this.cliServer, this.queryRunner, diff --git a/extensions/ql-vscode/src/model-editor/model-editor-view.ts b/extensions/ql-vscode/src/model-editor/model-editor-view.ts index 1d32867572b..57dca0991c5 100644 --- a/extensions/ql-vscode/src/model-editor/model-editor-view.ts +++ b/extensions/ql-vscode/src/model-editor/model-editor-view.ts @@ -29,7 +29,7 @@ import type { } from "../databases/local-databases"; import type { CodeQLCliServer } from "../codeql-cli/cli"; import { asError, assertNever, getErrorMessage } from "../common/helpers-pure"; -import { DatabaseFetcher } from "../databases/database-fetcher"; +import type { DatabaseFetcher } from "../databases/database-fetcher"; import type { App } from "../common/app"; import { redactableError } from "../common/errors"; import { @@ -86,6 +86,7 @@ export class ModelEditorView extends AbstractWebview< private readonly modelingEvents: ModelingEvents, private readonly modelConfig: ModelConfigListener, private readonly databaseManager: DatabaseManager, + private readonly databaseFetcher: DatabaseFetcher, private readonly variantAnalysisManager: VariantAnalysisManager, private readonly cliServer: CodeQLCliServer, private readonly queryRunner: QueryRunner, @@ -848,6 +849,7 @@ export class ModelEditorView extends AbstractWebview< this.modelingEvents, this.modelConfig, this.databaseManager, + this.databaseFetcher, this.variantAnalysisManager, this.cliServer, this.queryRunner, @@ -916,13 +918,7 @@ export class ModelEditorView extends AbstractWebview< // the user to import the library database. We need to have the database // imported to the query server, so we need to register it to our workspace. const makeSelected = false; - const databaseFetcher = new DatabaseFetcher( - this.app, - this.databaseManager, - this.app.workspaceStoragePath ?? this.app.globalStoragePath, - this.cliServer, - ); - const addedDatabase = await databaseFetcher.promptImportGithubDatabase( + const addedDatabase = await this.databaseFetcher.promptImportGithubDatabase( progress, this.databaseItem.language, undefined, diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/github-databases-module.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/github-databases-module.test.ts index 18607aab28e..bf22c35c9eb 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/github-databases-module.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/github-databases-module.test.ts @@ -4,7 +4,6 @@ import { createMockApp } from "../../../../__mocks__/appMock"; import type { App } from "../../../../../src/common/app"; import type { DatabaseManager } from "../../../../../src/databases/local-databases"; import { mockEmptyDatabaseManager } from "../../query-testing/test-runner-helpers"; -import type { CodeQLCliServer } from "../../../../../src/codeql-cli/cli"; import { mockDatabaseItem, mockedObject } from "../../../utils/mocking.helpers"; import type { GitHubDatabaseConfig } from "../../../../../src/config"; import { GitHubDatabasesModule } from "../../../../../src/databases/github-databases"; @@ -16,15 +15,13 @@ import * as githubDatabasesApi from "../../../../../src/databases/github-databas import * as githubDatabasesDownload from "../../../../../src/databases/github-databases/download"; import * as githubDatabasesUpdates from "../../../../../src/databases/github-databases/updates"; import type { DatabaseUpdate } from "../../../../../src/databases/github-databases/updates"; -import { DatabaseFetcher } from "../../../../../src/databases/database-fetcher"; +import type { DatabaseFetcher } from "../../../../../src/databases/database-fetcher"; describe("GitHubDatabasesModule", () => { describe("promptGitHubRepositoryDownload", () => { let app: App; let databaseManager: DatabaseManager; - let databaseStoragePath: string; - let cliServer: CodeQLCliServer; - let databaseFetcher: DatabaseFetcher; + const databaseFetcher = mockedObject({}); let config: GitHubDatabaseConfig; let gitHubDatabasesModule: GitHubDatabasesModule; @@ -66,14 +63,6 @@ describe("GitHubDatabasesModule", () => { beforeEach(() => { app = createMockApp(); databaseManager = mockEmptyDatabaseManager(); - databaseStoragePath = "/a/b/some-path"; - cliServer = mockedObject({}); - databaseFetcher = new DatabaseFetcher( - app, - databaseManager, - databaseStoragePath, - cliServer, - ); config = mockedObject({ download: "ask", update: "ask", diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/local-databases-ui.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/local-databases-ui.test.ts index 17470c1660d..90a72495e4d 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/local-databases-ui.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/local-databases-ui.test.ts @@ -20,6 +20,7 @@ import { testDisposeHandler } from "../../test-dispose-handler"; import { createMockApp } from "../../../__mocks__/appMock"; import { QueryLanguage } from "../../../../src/common/query-language"; import { mockedQuickPickItem, mockedObject } from "../../utils/mocking.helpers"; +import type { DatabaseFetcher } from "../../../../src/databases/database-fetcher"; describe("local-databases-ui", () => { const storageDir = dirSync({ unsafeCleanup: true }).name; @@ -104,6 +105,7 @@ describe("local-databases-ui", () => { }, setCurrentDatabaseItem: () => {}, } as any, + mockedObject({}), { onLanguageContextChanged: () => { /**/ @@ -142,6 +144,7 @@ describe("local-databases-ui", () => { setCurrentDatabaseItem: () => {}, currentDatabaseItem: { databaseUri: Uri.file(db1) }, } as any, + mockedObject({}), { onLanguageContextChanged: () => { /**/ @@ -178,6 +181,7 @@ describe("local-databases-ui", () => { const databaseUI = new DatabaseUI( app, databaseManager, + mockedObject({}), { onLanguageContextChanged: () => { /**/ diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/model-editor-view.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/model-editor-view.test.ts index e410c5ad156..c6a3237be6c 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/model-editor-view.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/model-editor-view.test.ts @@ -13,6 +13,7 @@ import type { ModelConfigListener } from "../../../../src/config"; import { createMockModelingEvents } from "../../../__mocks__/model-editor/modelingEventsMock"; import { QueryLanguage } from "../../../../src/common/query-language"; import type { VariantAnalysisManager } from "../../../../src/variant-analysis/variant-analysis-manager"; +import type { DatabaseFetcher } from "../../../../src/databases/database-fetcher"; describe("ModelEditorView", () => { const app = createMockApp({}); @@ -22,6 +23,7 @@ describe("ModelEditorView", () => { onDidChangeConfiguration: jest.fn(), }); const databaseManager = mockEmptyDatabaseManager(); + const databaseFetcher = mockedObject({}); const variantAnalysisManager = mockedObject({}); const cliServer = mockedObject({}); const queryRunner = mockedObject({}); @@ -50,6 +52,7 @@ describe("ModelEditorView", () => { modelingEvents, modelConfig, databaseManager, + databaseFetcher, variantAnalysisManager, cliServer, queryRunner, From 55b9b2e89158f3dda37b5dddb4d34a975c803802 Mon Sep 17 00:00:00 2001 From: Robert Date: Thu, 7 Mar 2024 15:05:32 +0000 Subject: [PATCH 6/9] Use mocks instead of spies in tests --- .../skeleton-query-wizard.test.ts | 29 +++++------- .../github-databases/download.test.ts | 44 +++++++----------- .../github-databases/updates.test.ts | 45 ++++++++----------- 3 files changed, 46 insertions(+), 72 deletions(-) diff --git a/extensions/ql-vscode/test/vscode-tests/cli-integration/local-queries/skeleton-query-wizard.test.ts b/extensions/ql-vscode/test/vscode-tests/cli-integration/local-queries/skeleton-query-wizard.test.ts index ece55e4b07c..aacac5e7a10 100644 --- a/extensions/ql-vscode/test/vscode-tests/cli-integration/local-queries/skeleton-query-wizard.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/cli-integration/local-queries/skeleton-query-wizard.test.ts @@ -23,7 +23,7 @@ import type { DatabaseManager, FullDatabaseOptions, } from "../../../../src/databases/local-databases"; -import { DatabaseFetcher } from "../../../../src/databases/database-fetcher"; +import type { DatabaseFetcher } from "../../../../src/databases/database-fetcher"; import { createMockDB } from "../../../factories/databases/databases"; import { asError } from "../../../../src/common/helpers-pure"; import { Setting } from "../../../../src/config"; @@ -56,9 +56,7 @@ describe("SkeletonQueryWizard", () => { let createExampleQlFileSpy: jest.SpiedFunction< typeof QlPackGenerator.prototype.createExampleQlFile >; - let promptImportGithubDatabaseSpy: jest.SpiedFunction< - DatabaseFetcher["promptImportGithubDatabase"] - >; + let promptImportGithubDatabaseMock: jest.Mock; let openTextDocumentSpy: jest.SpiedFunction< typeof workspace.openTextDocument >; @@ -113,12 +111,10 @@ describe("SkeletonQueryWizard", () => { }, ] as WorkspaceFolder[]); - databaseFetcher = new DatabaseFetcher( - mockApp, - mockDatabaseManager, - storagePath, - mockCli, - ); + promptImportGithubDatabaseMock = jest.fn().mockReturnValue(undefined); + databaseFetcher = mockedObject({ + promptImportGithubDatabase: promptImportGithubDatabaseMock, + }); quickPickSpy = jest.spyOn(window, "showQuickPick").mockResolvedValueOnce( mockedQuickPickItem({ @@ -138,9 +134,6 @@ describe("SkeletonQueryWizard", () => { createExampleQlFileSpy = jest .spyOn(QlPackGenerator.prototype, "createExampleQlFile") .mockResolvedValue(undefined); - promptImportGithubDatabaseSpy = jest - .spyOn(databaseFetcher, "promptImportGithubDatabase") - .mockResolvedValue(undefined); openTextDocumentSpy = jest .spyOn(workspace, "openTextDocument") .mockResolvedValue({} as TextDocument); @@ -203,7 +196,7 @@ describe("SkeletonQueryWizard", () => { title: "Download database", }), ); - expect(promptImportGithubDatabaseSpy).not.toHaveBeenCalled(); + expect(promptImportGithubDatabaseMock).not.toHaveBeenCalled(); }); it("should download database for selected language when selecting download in prompt", async () => { @@ -220,7 +213,7 @@ describe("SkeletonQueryWizard", () => { await wizard.execute(); await wizard.waitForDownload(); - expect(promptImportGithubDatabaseSpy).toHaveBeenCalled(); + expect(promptImportGithubDatabaseMock).toHaveBeenCalled(); }); it("should open the query file", async () => { @@ -329,7 +322,7 @@ describe("SkeletonQueryWizard", () => { it("should not download a new database for language", async () => { await wizard.execute(); - expect(promptImportGithubDatabaseSpy).not.toHaveBeenCalled(); + expect(promptImportGithubDatabaseMock).not.toHaveBeenCalled(); }); it("should not select the database", async () => { @@ -378,7 +371,7 @@ describe("SkeletonQueryWizard", () => { it("should not download a new database for language", async () => { await wizard.execute(); - expect(promptImportGithubDatabaseSpy).not.toHaveBeenCalled(); + expect(promptImportGithubDatabaseMock).not.toHaveBeenCalled(); }); it("should select an existing database", async () => { @@ -426,7 +419,7 @@ describe("SkeletonQueryWizard", () => { await wizard.execute(); await wizard.waitForDownload(); - expect(promptImportGithubDatabaseSpy).toHaveBeenCalled(); + expect(promptImportGithubDatabaseMock).toHaveBeenCalled(); }); }); }); diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/download.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/download.test.ts index b38c974cc24..96018ea6ba0 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/download.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/download.test.ts @@ -10,10 +10,9 @@ import { askForGitHubDatabaseDownload, downloadDatabaseFromGitHub, } from "../../../../../src/databases/github-databases/download"; -import type { DatabaseManager } from "../../../../../src/databases/local-databases"; +import type { DatabaseItem } from "../../../../../src/databases/local-databases"; import type { GitHubDatabaseConfig } from "../../../../../src/config"; -import type { CodeQLCliServer } from "../../../../../src/codeql-cli/cli"; -import { DatabaseFetcher } from "../../../../../src/databases/database-fetcher"; +import type { DatabaseFetcher } from "../../../../../src/databases/database-fetcher"; import * as dialog from "../../../../../src/common/vscode/dialog"; import type { CodeqlDatabase } from "../../../../../src/databases/github-databases/api"; import { createMockApp } from "../../../../__mocks__/appMock"; @@ -96,11 +95,8 @@ describe("downloadDatabaseFromGitHub", () => { let octokit: Octokit; const owner = "github"; const repo = "codeql"; - let databaseManager: DatabaseManager; let databaseFetcher: DatabaseFetcher; - const storagePath = "/a/b/c/d"; - let cliServer: CodeQLCliServer; const app = createMockApp(); let databases = [ @@ -117,20 +113,15 @@ describe("downloadDatabaseFromGitHub", () => { ]; let showQuickPickSpy: jest.SpiedFunction; - let downloadGitHubDatabaseFromUrlSpy: jest.SpiedFunction< - DatabaseFetcher["downloadGitHubDatabaseFromUrl"] - >; + let downloadGitHubDatabaseFromUrlMock: jest.Mock; beforeEach(() => { octokit = mockedObject({}); - databaseManager = mockedObject({}); - cliServer = mockedObject({}); - databaseFetcher = new DatabaseFetcher( - app, - databaseManager, - storagePath, - cliServer, - ); + + downloadGitHubDatabaseFromUrlMock = jest.fn().mockReturnValue(undefined); + databaseFetcher = mockedObject({ + downloadGitHubDatabaseFromUrl: downloadGitHubDatabaseFromUrlMock, + }); showQuickPickSpy = jest.spyOn(window, "showQuickPick").mockResolvedValue( mockedQuickPickItem([ @@ -139,9 +130,6 @@ describe("downloadDatabaseFromGitHub", () => { }), ]), ); - downloadGitHubDatabaseFromUrlSpy = jest - .spyOn(databaseFetcher, "downloadGitHubDatabaseFromUrl") - .mockResolvedValue(undefined); }); it("downloads the database", async () => { @@ -154,8 +142,8 @@ describe("downloadDatabaseFromGitHub", () => { app.commands, ); - expect(downloadGitHubDatabaseFromUrlSpy).toHaveBeenCalledTimes(1); - expect(downloadGitHubDatabaseFromUrlSpy).toHaveBeenCalledWith( + expect(downloadGitHubDatabaseFromUrlMock).toHaveBeenCalledTimes(1); + expect(downloadGitHubDatabaseFromUrlMock).toHaveBeenCalledWith( databases[0].url, databases[0].id, databases[0].created_at, @@ -213,8 +201,8 @@ describe("downloadDatabaseFromGitHub", () => { app.commands, ); - expect(downloadGitHubDatabaseFromUrlSpy).toHaveBeenCalledTimes(1); - expect(downloadGitHubDatabaseFromUrlSpy).toHaveBeenCalledWith( + expect(downloadGitHubDatabaseFromUrlMock).toHaveBeenCalledTimes(1); + expect(downloadGitHubDatabaseFromUrlMock).toHaveBeenCalledWith( databases[1].url, databases[1].id, databases[1].created_at, @@ -264,8 +252,8 @@ describe("downloadDatabaseFromGitHub", () => { app.commands, ); - expect(downloadGitHubDatabaseFromUrlSpy).toHaveBeenCalledTimes(2); - expect(downloadGitHubDatabaseFromUrlSpy).toHaveBeenCalledWith( + expect(downloadGitHubDatabaseFromUrlMock).toHaveBeenCalledTimes(2); + expect(downloadGitHubDatabaseFromUrlMock).toHaveBeenCalledWith( databases[0].url, databases[0].id, databases[0].created_at, @@ -277,7 +265,7 @@ describe("downloadDatabaseFromGitHub", () => { true, false, ); - expect(downloadGitHubDatabaseFromUrlSpy).toHaveBeenCalledWith( + expect(downloadGitHubDatabaseFromUrlMock).toHaveBeenCalledWith( databases[1].url, databases[1].id, databases[1].created_at, @@ -321,7 +309,7 @@ describe("downloadDatabaseFromGitHub", () => { app.commands, ); - expect(downloadGitHubDatabaseFromUrlSpy).not.toHaveBeenCalled(); + expect(downloadGitHubDatabaseFromUrlMock).not.toHaveBeenCalled(); }); }); }); diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/updates.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/updates.test.ts index d5b61957000..f4233d8f71b 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/updates.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/updates.test.ts @@ -8,10 +8,12 @@ import { mockedQuickPickItem, } from "../../../utils/mocking.helpers"; import type { CodeqlDatabase } from "../../../../../src/databases/github-databases/api"; -import type { DatabaseManager } from "../../../../../src/databases/local-databases"; +import type { + DatabaseItem, + DatabaseManager, +} from "../../../../../src/databases/local-databases"; import type { GitHubDatabaseConfig } from "../../../../../src/config"; -import type { CodeQLCliServer } from "../../../../../src/codeql-cli/cli"; -import { DatabaseFetcher } from "../../../../../src/databases/database-fetcher"; +import type { DatabaseFetcher } from "../../../../../src/databases/database-fetcher"; import * as dialog from "../../../../../src/common/vscode/dialog"; import type { DatabaseUpdate } from "../../../../../src/databases/github-databases/updates"; import { @@ -345,8 +347,6 @@ describe("downloadDatabaseUpdateFromGitHub", () => { const repo = "codeql"; let databaseManager: DatabaseManager; let databaseFetcher: DatabaseFetcher; - const storagePath = "/a/b/c/d"; - let cliServer: CodeQLCliServer; const app = createMockApp(); let updates: DatabaseUpdate[] = [ @@ -368,22 +368,18 @@ describe("downloadDatabaseUpdateFromGitHub", () => { ]; let showQuickPickSpy: jest.SpiedFunction; - let downloadGitHubDatabaseFromUrlSpy: jest.SpiedFunction< - DatabaseFetcher["downloadGitHubDatabaseFromUrl"] - >; + let downloadGitHubDatabaseFromUrlMock: jest.Mock; beforeEach(() => { octokit = mockedObject({}); databaseManager = mockedObject({ currentDatabaseItem: mockDatabaseItem(), }); - cliServer = mockedObject({}); - databaseFetcher = new DatabaseFetcher( - app, - databaseManager, - storagePath, - cliServer, - ); + + downloadGitHubDatabaseFromUrlMock = jest.fn().mockReturnValue(undefined); + databaseFetcher = mockedObject({ + downloadGitHubDatabaseFromUrl: downloadGitHubDatabaseFromUrlMock, + }); showQuickPickSpy = jest.spyOn(window, "showQuickPick").mockResolvedValue( mockedQuickPickItem([ @@ -392,9 +388,6 @@ describe("downloadDatabaseUpdateFromGitHub", () => { }), ]), ); - downloadGitHubDatabaseFromUrlSpy = jest - .spyOn(databaseFetcher, "downloadGitHubDatabaseFromUrl") - .mockResolvedValue(undefined); }); it("downloads the database", async () => { @@ -408,8 +401,8 @@ describe("downloadDatabaseUpdateFromGitHub", () => { app.commands, ); - expect(downloadGitHubDatabaseFromUrlSpy).toHaveBeenCalledTimes(1); - expect(downloadGitHubDatabaseFromUrlSpy).toHaveBeenCalledWith( + expect(downloadGitHubDatabaseFromUrlMock).toHaveBeenCalledTimes(1); + expect(downloadGitHubDatabaseFromUrlMock).toHaveBeenCalledWith( updates[0].database.url, updates[0].database.id, updates[0].database.created_at, @@ -483,8 +476,8 @@ describe("downloadDatabaseUpdateFromGitHub", () => { app.commands, ); - expect(downloadGitHubDatabaseFromUrlSpy).toHaveBeenCalledTimes(1); - expect(downloadGitHubDatabaseFromUrlSpy).toHaveBeenCalledWith( + expect(downloadGitHubDatabaseFromUrlMock).toHaveBeenCalledTimes(1); + expect(downloadGitHubDatabaseFromUrlMock).toHaveBeenCalledWith( updates[1].database.url, updates[1].database.id, updates[1].database.created_at, @@ -535,8 +528,8 @@ describe("downloadDatabaseUpdateFromGitHub", () => { app.commands, ); - expect(downloadGitHubDatabaseFromUrlSpy).toHaveBeenCalledTimes(2); - expect(downloadGitHubDatabaseFromUrlSpy).toHaveBeenCalledWith( + expect(downloadGitHubDatabaseFromUrlMock).toHaveBeenCalledTimes(2); + expect(downloadGitHubDatabaseFromUrlMock).toHaveBeenCalledWith( updates[0].database.url, updates[0].database.id, updates[0].database.created_at, @@ -548,7 +541,7 @@ describe("downloadDatabaseUpdateFromGitHub", () => { false, false, ); - expect(downloadGitHubDatabaseFromUrlSpy).toHaveBeenCalledWith( + expect(downloadGitHubDatabaseFromUrlMock).toHaveBeenCalledWith( updates[1].database.url, updates[1].database.id, updates[1].database.created_at, @@ -593,7 +586,7 @@ describe("downloadDatabaseUpdateFromGitHub", () => { app.commands, ); - expect(downloadGitHubDatabaseFromUrlSpy).not.toHaveBeenCalled(); + expect(downloadGitHubDatabaseFromUrlMock).not.toHaveBeenCalled(); }); }); }); From 834b6a649ed0d8bbd1781d737acab08f43d243f1 Mon Sep 17 00:00:00 2001 From: Robert Date: Wed, 13 Mar 2024 18:06:44 +0000 Subject: [PATCH 7/9] Use jest.MockedFunction --- .../local-queries/skeleton-query-wizard.test.ts | 4 +++- .../databases/github-databases/download.test.ts | 5 +++-- .../databases/github-databases/updates.test.ts | 9 ++++----- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/extensions/ql-vscode/test/vscode-tests/cli-integration/local-queries/skeleton-query-wizard.test.ts b/extensions/ql-vscode/test/vscode-tests/cli-integration/local-queries/skeleton-query-wizard.test.ts index aacac5e7a10..9076ec9bed4 100644 --- a/extensions/ql-vscode/test/vscode-tests/cli-integration/local-queries/skeleton-query-wizard.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/cli-integration/local-queries/skeleton-query-wizard.test.ts @@ -56,7 +56,9 @@ describe("SkeletonQueryWizard", () => { let createExampleQlFileSpy: jest.SpiedFunction< typeof QlPackGenerator.prototype.createExampleQlFile >; - let promptImportGithubDatabaseMock: jest.Mock; + let promptImportGithubDatabaseMock: jest.MockedFunction< + DatabaseFetcher["promptImportGithubDatabase"] + >; let openTextDocumentSpy: jest.SpiedFunction< typeof workspace.openTextDocument >; diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/download.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/download.test.ts index 96018ea6ba0..0803e8baa34 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/download.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/download.test.ts @@ -10,7 +10,6 @@ import { askForGitHubDatabaseDownload, downloadDatabaseFromGitHub, } from "../../../../../src/databases/github-databases/download"; -import type { DatabaseItem } from "../../../../../src/databases/local-databases"; import type { GitHubDatabaseConfig } from "../../../../../src/config"; import type { DatabaseFetcher } from "../../../../../src/databases/database-fetcher"; import * as dialog from "../../../../../src/common/vscode/dialog"; @@ -113,7 +112,9 @@ describe("downloadDatabaseFromGitHub", () => { ]; let showQuickPickSpy: jest.SpiedFunction; - let downloadGitHubDatabaseFromUrlMock: jest.Mock; + let downloadGitHubDatabaseFromUrlMock: jest.MockedFunction< + DatabaseFetcher["downloadGitHubDatabaseFromUrl"] + >; beforeEach(() => { octokit = mockedObject({}); diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/updates.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/updates.test.ts index f4233d8f71b..64ce5c54693 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/updates.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-databases/updates.test.ts @@ -8,10 +8,7 @@ import { mockedQuickPickItem, } from "../../../utils/mocking.helpers"; import type { CodeqlDatabase } from "../../../../../src/databases/github-databases/api"; -import type { - DatabaseItem, - DatabaseManager, -} from "../../../../../src/databases/local-databases"; +import type { DatabaseManager } from "../../../../../src/databases/local-databases"; import type { GitHubDatabaseConfig } from "../../../../../src/config"; import type { DatabaseFetcher } from "../../../../../src/databases/database-fetcher"; import * as dialog from "../../../../../src/common/vscode/dialog"; @@ -368,7 +365,9 @@ describe("downloadDatabaseUpdateFromGitHub", () => { ]; let showQuickPickSpy: jest.SpiedFunction; - let downloadGitHubDatabaseFromUrlMock: jest.Mock; + let downloadGitHubDatabaseFromUrlMock: jest.MockedFunction< + DatabaseFetcher["downloadGitHubDatabaseFromUrl"] + >; beforeEach(() => { octokit = mockedObject({}); From ff889b73e10af3dd3889acb946060cf722a3df53 Mon Sep 17 00:00:00 2001 From: Robert Date: Wed, 13 Mar 2024 18:09:51 +0000 Subject: [PATCH 8/9] Make documentation of fields more consistent --- extensions/ql-vscode/src/databases/database-fetcher.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/extensions/ql-vscode/src/databases/database-fetcher.ts b/extensions/ql-vscode/src/databases/database-fetcher.ts index 7b1f6aeb3f4..1299ab317d6 100644 --- a/extensions/ql-vscode/src/databases/database-fetcher.ts +++ b/extensions/ql-vscode/src/databases/database-fetcher.ts @@ -102,6 +102,7 @@ export class DatabaseFetcher { * * @param progress the progress callback * @param language the language to download. If undefined, the user will be prompted to choose a language. + * @param suggestedRepoNwo the suggested value to use when prompting for a github repo * @param makeSelected make the new database selected in the databases panel (default: true) * @param addSourceArchiveFolder whether to add a workspace folder containing the source archive to the workspace */ @@ -270,6 +271,7 @@ export class DatabaseFetcher { * Imports a database from a local archive. * * @param databaseUrl the file url of the archive to import + * @param progress the progress callback */ public async importArchiveDatabase( databaseUrl: string, From 7d259a9322728a3aa06ca6efcfdadf96d85085fe Mon Sep 17 00:00:00 2001 From: Robert Date: Thu, 28 Mar 2024 17:51:22 +0000 Subject: [PATCH 9/9] Fix dodgy imports --- .../src/databases/local-databases/database-contents.ts | 2 +- .../ql-vscode/src/databases/local-databases/database-manager.ts | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/extensions/ql-vscode/src/databases/local-databases/database-contents.ts b/extensions/ql-vscode/src/databases/local-databases/database-contents.ts index 14672c5c265..8d74fc071f4 100644 --- a/extensions/ql-vscode/src/databases/local-databases/database-contents.ts +++ b/extensions/ql-vscode/src/databases/local-databases/database-contents.ts @@ -1,5 +1,5 @@ import { pathExists, remove } from "fs-extra"; -import { join } from "path/posix"; +import { join } from "path"; import type { Uri } from "vscode"; import { zip } from "zip-a-folder"; diff --git a/extensions/ql-vscode/src/databases/local-databases/database-manager.ts b/extensions/ql-vscode/src/databases/local-databases/database-manager.ts index fba58f4e4d9..b766fae7d2d 100644 --- a/extensions/ql-vscode/src/databases/local-databases/database-manager.ts +++ b/extensions/ql-vscode/src/databases/local-databases/database-manager.ts @@ -43,7 +43,6 @@ import { DatabaseResolver } from "./database-resolver"; import { telemetryListener } from "../../common/vscode/telemetry"; import type { LanguageContextStore } from "../../language-context-store"; import type { DatabaseOrigin } from "./database-origin"; -import {} from "../database-fetcher"; import { ensureZippedSourceLocation } from "./database-contents"; /**