Merge pull request #3433 from github/aeisenberg/re-import-test-db

Import testproj databases into workspace storage
This commit is contained in:
Andrew Eisenberg
2024-03-13 12:34:01 -07:00
committed by GitHub
12 changed files with 320 additions and 35 deletions

View File

@@ -2,6 +2,8 @@
## [UNRELEASED]
- Databases created from [CodeQL test cases](https://docs.github.com/en/code-security/codeql-cli/using-the-advanced-functionality-of-the-codeql-cli/testing-custom-queries) are now copied into a shared VS Code storage location. This avoids a bug where re-running test cases would fail if the test's database is already imported into the workspace. [#3433](https://github.com/github/vscode-codeql/pull/3433)
## 1.12.3 - 29 February 2024
- Update variant analysis view to show when cancelation is in progress. [#3405](https://github.com/github/vscode-codeql/pull/3405)

View File

@@ -738,6 +738,10 @@
"command": "codeQL.setCurrentDatabase",
"title": "CodeQL: Set Current Database"
},
{
"command": "codeQL.importTestDatabase",
"title": "CodeQL: (Re-)Import Test Database"
},
{
"command": "codeQL.getCurrentDatabase",
"title": "CodeQL: Get Current Database"
@@ -1322,7 +1326,12 @@
{
"command": "codeQL.setCurrentDatabase",
"group": "9_qlCommands",
"when": "resourceScheme == codeql-zip-archive || explorerResourceIsFolder || resourceExtname == .zip"
"when": "resourceExtname != .testproj && (resourceScheme == codeql-zip-archive || explorerResourceIsFolder || resourceExtname == .zipz)"
},
{
"command": "codeQL.importTestDatabase",
"group": "9_qlCommands",
"when": "explorerResourceIsFolder && resourceExtname == .testproj"
},
{
"command": "codeQL.viewAstContextExplorer",
@@ -1476,6 +1485,10 @@
"command": "codeQL.setCurrentDatabase",
"when": "false"
},
{
"command": "codeQL.importTestDatabase",
"when": "false"
},
{
"command": "codeQL.getCurrentDatabase",
"when": "false"
@@ -2018,7 +2031,6 @@
"@types/tar-stream": "^3.1.3",
"@types/through2": "^2.0.36",
"@types/tmp": "^0.2.6",
"@types/unzipper": "^0.10.1",
"@types/vscode": "^1.82.0",
"@types/yauzl": "^2.10.3",
"@typescript-eslint/eslint-plugin": "^6.19.0",

View File

@@ -220,6 +220,7 @@ export type LocalDatabasesCommands = {
// Explorer context menu
"codeQL.setCurrentDatabase": (uri: Uri) => Promise<void>;
"codeQL.importTestDatabase": (uri: Uri) => Promise<void>;
// Database panel view title commands
"codeQLDatabases.chooseDatabaseFolder": () => Promise<void>;

View File

@@ -11,6 +11,7 @@ import {
createWriteStream,
remove,
readdir,
copy,
} from "fs-extra";
import { basename, join } from "path";
import type { Octokit } from "@octokit/rest";
@@ -64,7 +65,7 @@ export async function promptImportInternetDatabase(
validateUrl(databaseUrl);
const item = await databaseArchiveFetcher(
const item = await fetchDatabaseToWorkspaceStorage(
databaseUrl,
{},
databaseManager,
@@ -258,7 +259,7 @@ export async function downloadGitHubDatabaseFromUrl(
* We only need the actual token string.
*/
const octokitToken = ((await octokit.auth()) as { token: string })?.token;
return await databaseArchiveFetcher(
return await fetchDatabaseToWorkspaceStorage(
databaseUrl,
{
Accept: "application/zip",
@@ -282,14 +283,15 @@ export async function downloadGitHubDatabaseFromUrl(
}
/**
* Imports a database from a local archive.
* Imports a database from a local archive or a test database that is in a folder
* ending with `.testproj`.
*
* @param databaseUrl the file url of the archive to import
* @param databaseUrl the file url of the archive or directory to import
* @param databaseManager the DatabaseManager
* @param storagePath where to store the unzipped database.
* @param cli the CodeQL CLI server
*/
export async function importArchiveDatabase(
export async function importLocalDatabase(
commandManager: AppCommandManager,
databaseUrl: string,
databaseManager: DatabaseManager,
@@ -298,16 +300,17 @@ export async function importArchiveDatabase(
cli: CodeQLCliServer,
): Promise<DatabaseItem | undefined> {
try {
const item = await databaseArchiveFetcher(
const origin: DatabaseOrigin = {
type: databaseUrl.endsWith(".testproj") ? "testproj" : "archive",
path: Uri.parse(databaseUrl).fsPath,
};
const item = await fetchDatabaseToWorkspaceStorage(
databaseUrl,
{},
databaseManager,
storagePath,
undefined,
{
type: "archive",
path: databaseUrl,
},
origin,
progress,
cli,
);
@@ -315,7 +318,9 @@ export async function importArchiveDatabase(
await commandManager.execute("codeQLDatabases.focus");
void showAndLogInformationMessage(
extLogger,
"Database unzipped and imported successfully.",
origin.type === "testproj"
? "Test database imported successfully."
: "Database unzipped and imported successfully.",
);
}
return item;
@@ -332,10 +337,10 @@ export async function importArchiveDatabase(
}
/**
* Fetches an archive database. The database might be on the internet
* Fetches a database into workspace storage. The database might be on the internet
* or in the local filesystem.
*
* @param databaseUrl URL from which to grab the database
* @param databaseUrl URL from which to grab the database. This could be a local archive file, a local directory, or a remote URL.
* @param requestHeaders Headers to send with the request
* @param databaseManager the DatabaseManager
* @param storagePath where to store the unzipped database.
@@ -346,7 +351,7 @@ export async function importArchiveDatabase(
* @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(
async function fetchDatabaseToWorkspaceStorage(
databaseUrl: string,
requestHeaders: { [key: string]: string },
databaseManager: DatabaseManager,
@@ -374,7 +379,11 @@ async function databaseArchiveFetcher(
);
if (isFile(databaseUrl)) {
await readAndUnzip(databaseUrl, unzipPath, cli, progress);
if (origin.type === "testproj") {
await copyDatabase(databaseUrl, unzipPath, progress);
} else {
await readAndUnzip(databaseUrl, unzipPath, cli, progress);
}
} else {
await fetchAndUnzip(databaseUrl, requestHeaders, unzipPath, cli, progress);
}
@@ -438,6 +447,8 @@ async function getStorageFolder(
lastName = basename(url.path).substring(0, 250);
if (lastName.endsWith(".zip")) {
lastName = lastName.substring(0, lastName.length - 4);
} else if (lastName.endsWith(".testproj")) {
lastName = lastName.substring(0, lastName.length - 9);
}
}
@@ -484,6 +495,26 @@ function validateUrl(databaseUrl: string) {
}
}
/**
* Copies a database folder from the file system into the workspace storage.
* @param scrDirURL the original location of the database as a URL string
* @param destDir the location to copy the database to. This should be a folder in the workspace storage.
* @param progress callback to send progress messages to
*/
async function copyDatabase(
srcDirURL: string,
destDir: string,
progress?: ProgressCallback,
) {
progress?.({
maxStep: 10,
step: 9,
message: `Copying database ${basename(destDir)} into the workspace`,
});
await ensureDir(destDir);
await copy(Uri.parse(srcDirURL).fsPath, destDir);
}
async function readAndUnzip(
zipUrl: string,
unzipPath: string,

View File

@@ -43,7 +43,7 @@ import {
showAndLogErrorMessage,
} from "../common/logging";
import {
importArchiveDatabase,
importLocalDatabase,
promptImportGithubDatabase,
promptImportInternetDatabase,
} from "./database-fetcher";
@@ -145,7 +145,8 @@ class DatabaseTreeDataProvider
item.iconPath = new ThemeIcon("error", new ThemeColor("errorForeground"));
}
item.tooltip = element.databaseUri.fsPath;
item.description = element.language;
item.description =
element.language + (element.origin?.type === "testproj" ? " (test)" : "");
return item;
}
@@ -281,6 +282,7 @@ export class DatabaseUI extends DisposableObject {
this.handleChooseDatabaseInternet.bind(this),
"codeQL.chooseDatabaseGithub": this.handleChooseDatabaseGithub.bind(this),
"codeQL.setCurrentDatabase": this.handleSetCurrentDatabase.bind(this),
"codeQL.importTestDatabase": this.handleImportTestDatabase.bind(this),
"codeQL.setDefaultTourDatabase":
this.handleSetDefaultTourDatabase.bind(this),
"codeQL.upgradeCurrentDatabase":
@@ -712,7 +714,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 importLocalDatabase(
this.app.commands,
uri.toString(true),
this.databaseManager,
@@ -740,6 +742,60 @@ export class DatabaseUI extends DisposableObject {
);
}
private async handleImportTestDatabase(uri: Uri): Promise<void> {
return withProgress(
async (progress) => {
try {
if (!uri.path.endsWith(".testproj")) {
throw new Error(
"Please select a valid test database to import. Test databases end with `.testproj`.",
);
}
// Check if the database is already in the workspace. If
// so, delete it first before importing the new one.
const existingItem = this.databaseManager.findTestDatabase(uri);
const baseName = basename(uri.fsPath);
if (existingItem !== undefined) {
progress({
maxStep: 9,
step: 1,
message: `Removing existing test database ${baseName}`,
});
await this.databaseManager.removeDatabaseItem(existingItem);
}
await importLocalDatabase(
this.app.commands,
uri.toString(true),
this.databaseManager,
this.storagePath,
progress,
this.queryServer.cliServer,
);
if (existingItem !== undefined) {
progress({
maxStep: 9,
step: 9,
message: `Successfully re-imported ${baseName}`,
});
}
} catch (e) {
// rethrow and let this be handled by default error handling.
throw new Error(
`Could not set database to ${basename(
uri.fsPath,
)}. Reason: ${getErrorMessage(e)}`,
);
}
},
{
title: "(Re-)importing test database from directory",
},
);
}
private async handleRemoveDatabase(
databaseItems: DatabaseItem[],
): Promise<void> {
@@ -959,7 +1015,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 importLocalDatabase(
this.app.commands,
uri.toString(true),
this.databaseManager,

View File

@@ -18,7 +18,10 @@ import {
import { join } from "path";
import type { FullDatabaseOptions } from "./database-options";
import { DatabaseItemImpl } from "./database-item-impl";
import { showNeverAskAgainDialog } from "../../common/vscode/dialog";
import {
showBinaryChoiceDialog,
showNeverAskAgainDialog,
} from "../../common/vscode/dialog";
import {
getFirstWorkspaceFolder,
isFolderAlreadyInWorkspace,
@@ -32,7 +35,7 @@ import { QlPackGenerator } from "../../local-queries/qlpack-generator";
import { asError, getErrorMessage } from "../../common/helpers-pure";
import type { DatabaseItem, PersistedDatabaseItem } from "./database-item";
import { redactableError } from "../../common/errors";
import { remove } from "fs-extra";
import { copy, remove, stat } from "fs-extra";
import { containsPath } from "../../common/files";
import type { DatabaseChangedEvent } from "./database-events";
import { DatabaseEventKind } from "./database-events";
@@ -120,6 +123,7 @@ export class DatabaseManager extends DisposableObject {
super();
qs.onStart(this.reregisterDatabases.bind(this));
qs.onQueryRunStarting(this.maybeReimportTestDatabase.bind(this));
this.push(
this.languageContext.onLanguageContextChanged(async () => {
@@ -165,6 +169,99 @@ export class DatabaseManager extends DisposableObject {
);
}
/**
* Finds a test database that was originally imported from `uri`.
* A test database is creeated by the `codeql test run` command
* and ends with `.testproj`.
* @param uri The original location of the database
* @returns The first database item found that matches the uri
*/
public findTestDatabase(uri: vscode.Uri): DatabaseItem | undefined {
const originPath = uri.fsPath;
for (const item of this._databaseItems) {
if (item.origin?.type === "testproj" && item.origin.path === originPath) {
return item;
}
}
return undefined;
}
public async maybeReimportTestDatabase(
databaseUri: vscode.Uri,
forceImport = false,
): Promise<void> {
const res = await this.isTestDatabaseOutdated(databaseUri);
if (!res) {
return;
}
const doit =
forceImport ||
(await showBinaryChoiceDialog(
"This test database is outdated. Do you want to reimport it?",
));
if (doit) {
await this.reimportTestDatabase(databaseUri);
}
}
/**
* Checks if the origin of the imported database is newer.
* The imported database must be a test database.
* @param databaseUri the URI of the imported database to check
* @returns true if both databases exist and the origin database is newer.
*/
private async isTestDatabaseOutdated(
databaseUri: vscode.Uri,
): Promise<boolean> {
const dbItem = this.findDatabaseItem(databaseUri);
if (dbItem === undefined || dbItem.origin?.type !== "testproj") {
return false;
}
// Compare timestmps of the codeql-database.yml files of the original and the
// imported databases.
const originDbYml = join(dbItem.origin.path, "codeql-database.yml");
const importedDbYml = join(
dbItem.databaseUri.fsPath,
"codeql-database.yml",
);
try {
const originStat = await stat(originDbYml);
const importedStat = await stat(importedDbYml);
return originStat.mtimeMs > importedStat.mtimeMs;
} catch (e) {
// If either of the files does not exist, we assume the origin is newer.
// This shouldn't happen unless the user manually deleted one of the files.
return true;
}
}
/**
* Reimport the specified imported database from its origin.
* The imported databsae must be a testproj database.
*
* @param databaseUri the URI of the imported database to reimport
*/
private async reimportTestDatabase(databaseUri: vscode.Uri): Promise<void> {
const dbItem = this.findDatabaseItem(databaseUri);
if (dbItem === undefined || dbItem.origin?.type !== "testproj") {
throw new Error(`Database ${databaseUri} is not a testproj.`);
}
await this.removeDatabaseItem(dbItem);
await copy(dbItem.origin.path, databaseUri.fsPath);
const newDbItem = new DatabaseItemImpl(databaseUri, dbItem.contents, {
dateAdded: Date.now(),
language: dbItem.language,
origin: dbItem.origin,
extensionManagedLocation: dbItem.extensionManagedLocation,
});
await this.addDatabaseItem(newDbItem);
await this.setCurrentDatabaseItem(newDbItem);
}
/**
* Adds a {@link DatabaseItem} to the list of open databases, if that database is not already on
* the list.

View File

@@ -24,9 +24,15 @@ interface DatabaseOriginDebugger {
type: "debugger";
}
interface DatabaseOriginTestProj {
type: "testproj";
path: string;
}
export type DatabaseOrigin =
| DatabaseOriginFolder
| DatabaseOriginArchive
| DatabaseOriginGitHub
| DatabaseOriginInternet
| DatabaseOriginDebugger;
| DatabaseOriginDebugger
| DatabaseOriginTestProj;

View File

@@ -1,4 +1,4 @@
import { window } from "vscode";
import { window, Uri } from "vscode";
import type { CancellationToken, MessageItem } from "vscode";
import type { CodeQLCliServer } from "../codeql-cli/cli";
import type { ProgressCallback } from "../common/vscode/progress";
@@ -63,9 +63,22 @@ export interface CoreQueryRun {
export type CoreCompletedQuery = CoreQueryResults &
Omit<CoreQueryRun, "evaluate">;
type OnQueryRunStartingListener = (dbPath: Uri) => Promise<void>;
export class QueryRunner {
constructor(public readonly qs: QueryServerClient) {}
// Event handlers that get notified whenever a query is about to start running.
// Can't use vscode EventEmitters since they are not asynchronous.
private readonly onQueryRunStartingListeners: OnQueryRunStartingListener[] =
[];
public onQueryRunStarting(listener: OnQueryRunStartingListener) {
this.onQueryRunStartingListeners.push(listener);
}
private async fireQueryRunStarting(dbPath: Uri) {
await Promise.all(this.onQueryRunStartingListeners.map((l) => l(dbPath)));
}
get cliServer(): CodeQLCliServer {
return this.qs.cliServer;
}
@@ -138,6 +151,8 @@ export class QueryRunner {
templates: Record<string, string> | undefined,
logger: BaseLogger,
): Promise<CoreQueryResults> {
await this.fireQueryRunStarting(Uri.file(dbPath));
return await compileAndRunQueryAgainstDatabaseCore(
this.qs,
dbPath,

View File

@@ -4,7 +4,7 @@ import { Uri, window } from "vscode";
import type { CodeQLCliServer } from "../../../../src/codeql-cli/cli";
import type { DatabaseManager } from "../../../../src/databases/local-databases";
import {
importArchiveDatabase,
importLocalDatabase,
promptImportInternetDatabase,
} from "../../../../src/databases/database-fetcher";
import {
@@ -13,9 +13,11 @@ import {
DB_URL,
getActivatedExtension,
storagePath,
testprojLoc,
} from "../../global.helper";
import { createMockCommandManager } from "../../../__mocks__/commandsMock";
import { remove } from "fs-extra";
import { utimesSync } from "fs";
import { remove, existsSync } from "fs-extra";
/**
* Run various integration tests for databases
@@ -46,10 +48,10 @@ describe("database-fetcher", () => {
await remove(storagePath);
});
describe("importArchiveDatabase", () => {
it("should add a database from a folder", async () => {
describe("importLocalDatabase", () => {
it("should add a database from an archive", async () => {
const uri = Uri.file(dbLoc);
let dbItem = await importArchiveDatabase(
let dbItem = await importLocalDatabase(
createMockCommandManager(),
uri.toString(true),
databaseManager,
@@ -64,6 +66,42 @@ describe("database-fetcher", () => {
expect(dbItem.name).toBe("db");
expect(dbItem.databaseUri.fsPath).toBe(join(storagePath, "db", "db"));
});
it("should import a testproj database", async () => {
let dbItem = await importLocalDatabase(
createMockCommandManager(),
Uri.file(testprojLoc).toString(true),
databaseManager,
storagePath,
progressCallback,
cli,
);
expect(dbItem).toBe(databaseManager.currentDatabaseItem);
expect(dbItem).toBe(databaseManager.databaseItems[0]);
expect(dbItem).toBeDefined();
dbItem = dbItem!;
expect(dbItem.name).toBe("db");
expect(dbItem.databaseUri.fsPath).toBe(join(storagePath, "db"));
// Now that we have fetched it. Check for re-importing
// Delete a file in the imported database and we can check if the file is recreated
const srczip = join(dbItem.databaseUri.fsPath, "src.zip");
await remove(srczip);
// Attempt 1: re-import database should be a no-op since timestamp of imported database is newer
await databaseManager.maybeReimportTestDatabase(dbItem.databaseUri);
expect(existsSync(srczip)).toBeFalsy();
// Attempt 3: re-import database should re-import the database after updating modified time
utimesSync(
join(testprojLoc, "codeql-database.yml"),
new Date(),
new Date(),
);
await databaseManager.maybeReimportTestDatabase(dbItem.databaseUri, true);
expect(existsSync(srczip)).toBeTruthy();
});
});
describe("promptImportInternetDatabase", () => {

View File

@@ -6,13 +6,18 @@ import {
beforeEachAction,
} from "../jest.activated-extension.setup";
import { createWriteStream, existsSync, mkdirpSync } from "fs-extra";
import { dirname } from "path";
import { DB_URL, dbLoc } from "../global.helper";
import { dirname, join } from "path";
import { DB_URL, dbLoc, testprojLoc } from "../global.helper";
import fetch from "node-fetch";
import { renameSync } from "fs";
import { unzipToDirectoryConcurrently } from "../../../src/common/unzip-concurrently";
import { platform } from "os";
import { sleep } from "../../../src/common/time";
beforeAll(async () => {
// ensure the test database is downloaded
mkdirpSync(dirname(dbLoc));
const dbParentDir = dirname(dbLoc);
mkdirpSync(dbParentDir);
if (!existsSync(dbLoc)) {
console.log(`Downloading test database to ${dbLoc}`);
@@ -30,6 +35,19 @@ beforeAll(async () => {
});
}
// unzip the database from dbLoc to testprojLoc
if (!existsSync(testprojLoc)) {
console.log(`Unzipping test database to ${testprojLoc}`);
await unzipToDirectoryConcurrently(dbLoc, dbParentDir);
// On Windows, wait a few seconds to make sure all background processes
// release their lock on the files before renaming the directory.
if (platform() === "win32") {
await sleep(3000);
}
renameSync(join(dbParentDir, "db"), testprojLoc);
console.log("Unzip completed.");
}
await beforeAllAction();
});

View File

@@ -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 { importLocalDatabase } from "../../src/databases/database-fetcher";
import { createMockCommandManager } from "../__mocks__/commandsMock";
// This file contains helpers shared between tests that work with an activated extension.
@@ -21,6 +21,12 @@ export const dbLoc = join(
realpathSync(join(__dirname, "../../../")),
"build/tests/db.zip",
);
export const testprojLoc = join(
realpathSync(join(__dirname, "../../../")),
"build/tests/db.testproj",
);
// eslint-disable-next-line import/no-mutable-exports
export let storagePath: string;
@@ -34,7 +40,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 importLocalDatabase(
createMockCommandManager(),
uri.toString(true),
databaseManager,

View File

@@ -101,6 +101,9 @@ describe("local databases", () => {
onStart: () => {
/**/
},
onQueryRunStarting: () => {
/**/
},
}),
mockedObject<CodeQLCliServer>({
resolveDatabase: resolveDatabaseSpy,