From a0295d62f8e5f4724613e57433ddbf7266aff511 Mon Sep 17 00:00:00 2001 From: Koen Vlaswinkel Date: Mon, 20 Nov 2023 12:07:39 +0100 Subject: [PATCH 1/5] Add prompt for updating GitHub databases --- extensions/ql-vscode/package.json | 13 + extensions/ql-vscode/src/config.ts | 22 + .../src/databases/github-database-download.ts | 17 +- .../src/databases/github-database-module.ts | 59 +- .../src/databases/github-database-updates.ts | 205 +++++++ .../databases/github-database-updates.test.ts | 557 ++++++++++++++++++ 6 files changed, 858 insertions(+), 15 deletions(-) create mode 100644 extensions/ql-vscode/src/databases/github-database-updates.ts create mode 100644 extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-database-updates.test.ts diff --git a/extensions/ql-vscode/package.json b/extensions/ql-vscode/package.json index eca78aced94..834e784679a 100644 --- a/extensions/ql-vscode/package.json +++ b/extensions/ql-vscode/package.json @@ -441,6 +441,19 @@ "Never download a GitHub databases when a workspace is opened." ], "description": "Ask to download a GitHub database when a workspace is opened." + }, + "codeQL.githubDatabase.update": { + "type": "string", + "default": "ask", + "enum": [ + "ask", + "never" + ], + "enumDescriptions": [ + "Ask to download an updated GitHub database when a new version is available.", + "Never download an updated GitHub database when a new version is available." + ], + "description": "Ask to download an updated GitHub database when a new version is available." } } }, diff --git a/extensions/ql-vscode/src/config.ts b/extensions/ql-vscode/src/config.ts index 7a9170d6dc0..433e4909746 100644 --- a/extensions/ql-vscode/src/config.ts +++ b/extensions/ql-vscode/src/config.ts @@ -788,13 +788,23 @@ const GITHUB_DATABASE_DOWNLOAD = new Setting( const GitHubDatabaseDownloadValues = ["ask", "never"] as const; type GitHubDatabaseDownload = (typeof GitHubDatabaseDownloadValues)[number]; +const GITHUB_DATABASE_UPDATE = new Setting("update", GITHUB_DATABASE_SETTING); + +const GitHubDatabaseUpdateValues = ["ask", "never"] as const; +type GitHubDatabaseUpdate = (typeof GitHubDatabaseUpdateValues)[number]; + export interface GitHubDatabaseConfig { enable: boolean; download: GitHubDatabaseDownload; + update: GitHubDatabaseUpdate; setDownload( value: GitHubDatabaseDownload, target?: ConfigurationTarget, ): Promise; + setUpdate( + value: GitHubDatabaseUpdate, + target?: ConfigurationTarget, + ): Promise; } export class GitHubDatabaseConfigListener @@ -817,10 +827,22 @@ export class GitHubDatabaseConfigListener return GitHubDatabaseDownloadValues.includes(value) ? value : "ask"; } + public get update(): GitHubDatabaseUpdate { + const value = GITHUB_DATABASE_UPDATE.getValue(); + return GitHubDatabaseUpdateValues.includes(value) ? value : "ask"; + } + public async setDownload( value: GitHubDatabaseDownload, target: ConfigurationTarget = ConfigurationTarget.Workspace, ): Promise { await GITHUB_DATABASE_DOWNLOAD.updateValue(value, target); } + + public async setUpdate( + value: GitHubDatabaseUpdate, + target: ConfigurationTarget = ConfigurationTarget.Workspace, + ): Promise { + await GITHUB_DATABASE_UPDATE.updateValue(value, target); + } } diff --git a/extensions/ql-vscode/src/databases/github-database-download.ts b/extensions/ql-vscode/src/databases/github-database-download.ts index 4f24ef38c68..1a4852996b0 100644 --- a/extensions/ql-vscode/src/databases/github-database-download.ts +++ b/extensions/ql-vscode/src/databases/github-database-download.ts @@ -110,7 +110,7 @@ export async function downloadDatabaseFromGitHub( * * @param languages The languages to join. These should be language identifiers, such as `csharp`. */ -function joinLanguages(languages: string[]): string { +export function joinLanguages(languages: string[]): string { const languageDisplayNames = languages .map((language) => getLanguageDisplayName(language)) .sort(); @@ -130,8 +130,17 @@ function joinLanguages(languages: string[]): string { return result; } -async function promptForDatabases( +type PromptForDatabasesOptions = { + title?: string; + placeHolder?: string; +}; + +export async function promptForDatabases( databases: CodeqlDatabase[], + { + title = "Select databases to download", + placeHolder = "Databases found in this repository", + }: PromptForDatabasesOptions = {}, ): Promise { if (databases.length === 1) { return databases; @@ -152,8 +161,8 @@ async function promptForDatabases( .sort((a, b) => a.label.localeCompare(b.label)); const selectedItems = await window.showQuickPick(items, { - title: "Select databases to download", - placeHolder: "Databases found in this repository", + title, + placeHolder, ignoreFocusOut: true, canPickMany: true, }); diff --git a/extensions/ql-vscode/src/databases/github-database-module.ts b/extensions/ql-vscode/src/databases/github-database-module.ts index e6fe8d1b654..beab4fc4c8e 100644 --- a/extensions/ql-vscode/src/databases/github-database-module.ts +++ b/extensions/ql-vscode/src/databases/github-database-module.ts @@ -3,7 +3,7 @@ import { DisposableObject } from "../common/disposable-object"; import { App } from "../common/app"; import { findGitHubRepositoryForWorkspace } from "./github-repository-finder"; import { redactableError } from "../common/errors"; -import { asError, getErrorMessage } from "../common/helpers-pure"; +import { asError, assertNever, getErrorMessage } from "../common/helpers-pure"; import { askForGitHubDatabaseDownload, downloadDatabaseFromGitHub, @@ -12,6 +12,11 @@ import { GitHubDatabaseConfig, GitHubDatabaseConfigListener } from "../config"; import { DatabaseManager } from "./local-databases"; import { CodeQLCliServer } from "../codeql-cli/cli"; import { listDatabases, ListDatabasesResult } from "./github-database-api"; +import { + askForGitHubDatabaseUpdate, + downloadDatabaseUpdateFromGitHub, + isNewerDatabaseAvailable, +} from "./github-database-updates"; export class GithubDatabaseModule extends DisposableObject { private readonly config: GitHubDatabaseConfig; @@ -79,16 +84,6 @@ export class GithubDatabaseModule extends DisposableObject { const githubRepository = githubRepositoryResult.value; - const hasExistingDatabase = this.databaseManager.databaseItems.some( - (db) => - db.origin?.type === "github" && - db.origin.repository === - `${githubRepository.owner}/${githubRepository.name}`, - ); - if (hasExistingDatabase) { - return; - } - let result: ListDatabasesResult | undefined; try { result = await listDatabases( @@ -130,6 +125,48 @@ export class GithubDatabaseModule extends DisposableObject { return; } + const updateStatus = isNewerDatabaseAvailable( + databases, + githubRepository.owner, + githubRepository.name, + this.databaseManager, + ); + if (updateStatus.type === "upToDate") { + return; + } + + if (updateStatus.type === "updateAvailable") { + if (this.config.update === "never") { + return; + } + + if ( + !(await askForGitHubDatabaseUpdate( + updateStatus.databaseUpdates, + this.config, + )) + ) { + return; + } + + await downloadDatabaseUpdateFromGitHub( + octokit, + githubRepository.owner, + githubRepository.name, + updateStatus.databaseUpdates, + this.databaseManager, + this.databaseStoragePath, + this.cliServer, + this.app.commands, + ); + + return; + } + + if (updateStatus.type !== "noDatabase") { + assertNever(updateStatus); + } + // If the user already had an access token, first ask if they even want to download the DB. if (!promptedForCredentials) { if (!(await askForGitHubDatabaseDownload(databases, this.config))) { diff --git a/extensions/ql-vscode/src/databases/github-database-updates.ts b/extensions/ql-vscode/src/databases/github-database-updates.ts new file mode 100644 index 00000000000..12ad48186f5 --- /dev/null +++ b/extensions/ql-vscode/src/databases/github-database-updates.ts @@ -0,0 +1,205 @@ +import { CodeqlDatabase } from "./github-database-api"; +import { DatabaseItem, DatabaseManager } from "./local-databases"; +import { Octokit } from "@octokit/rest"; +import { CodeQLCliServer } from "../codeql-cli/cli"; +import { AppCommandManager } from "../common/commands"; +import { getLanguageDisplayName } from "../common/query-language"; +import { showNeverAskAgainDialog } from "../common/vscode/dialog"; +import { downloadGitHubDatabaseFromUrl } from "./database-fetcher"; +import { withProgress } from "../common/vscode/progress"; +import { window } from "vscode"; +import { GitHubDatabaseConfig } from "../config"; +import { joinLanguages, promptForDatabases } from "./github-database-download"; + +export type DatabaseUpdate = { + database: CodeqlDatabase; + databaseItem: DatabaseItem; +}; + +type DatabaseUpdateStatusUpdateAvailable = { + type: "updateAvailable"; + databaseUpdates: DatabaseUpdate[]; +}; + +type DatabaseUpdateStatusUpToDate = { + type: "upToDate"; +}; + +type DatabaseUpdateStatusNoDatabase = { + type: "noDatabase"; +}; + +type DatabaseUpdateStatus = + | DatabaseUpdateStatusUpdateAvailable + | DatabaseUpdateStatusUpToDate + | DatabaseUpdateStatusNoDatabase; + +export function isNewerDatabaseAvailable( + databases: CodeqlDatabase[], + owner: string, + name: string, + databaseManager: DatabaseManager, +): DatabaseUpdateStatus { + // Sorted by date added ascending + const existingDatabasesForRepository = databaseManager.databaseItems + .filter( + (db) => + db.origin?.type === "github" && + db.origin.repository === `${owner}/${name}`, + ) + .sort((a, b) => (a.dateAdded ?? 0) - (b.dateAdded ?? 0)); + + if (existingDatabasesForRepository.length === 0) { + return { + type: "noDatabase", + }; + } + + // Sort order is guaranteed by the sort call above. The newest database is the last one. + const newestExistingDatabasesByLanguage = new Map(); + for (const existingDatabase of existingDatabasesForRepository) { + newestExistingDatabasesByLanguage.set( + existingDatabase.language, + existingDatabase, + ); + } + + const databaseUpdates = Array.from(newestExistingDatabasesByLanguage.values()) + .map((newestExistingDatabase): DatabaseUpdate | null => { + const origin = newestExistingDatabase.origin; + if (origin?.type !== "github") { + return null; + } + + const matchingDatabase = databases.find( + (db) => db.language === newestExistingDatabase.language, + ); + if (!matchingDatabase) { + return null; + } + + if (matchingDatabase.commit_oid === origin.commitOid) { + return null; + } + + return { + database: matchingDatabase, + databaseItem: newestExistingDatabase, + }; + }) + .filter((update): update is DatabaseUpdate => update !== null) + .sort((a, b) => a.database.language.localeCompare(b.database.language)); + + if (databaseUpdates.length === 0) { + return { + type: "upToDate", + }; + } + + return { + type: "updateAvailable", + databaseUpdates, + }; +} + +export async function askForGitHubDatabaseUpdate( + updates: DatabaseUpdate[], + config: GitHubDatabaseConfig, +): Promise { + const languages = updates.map((update) => update.database.language); + + const message = + updates.length === 1 + ? `There is a newer ${getLanguageDisplayName( + languages[0], + )} CodeQL database available for this repository. Download the database update from GitHub?` + : `There are newer ${joinLanguages( + languages, + )} CodeQL databases available for this repository. Download the database updates from GitHub?`; + + const answer = await showNeverAskAgainDialog( + message, + false, + "Download", + "Not now", + "Never", + ); + + if (answer === "Not now" || answer === undefined) { + return false; + } + + if (answer === "Never") { + await config.setUpdate("never"); + return false; + } + + return true; +} + +export async function downloadDatabaseUpdateFromGitHub( + octokit: Octokit, + owner: string, + repo: string, + updates: DatabaseUpdate[], + databaseManager: DatabaseManager, + storagePath: string, + cliServer: CodeQLCliServer, + commandManager: AppCommandManager, +): Promise { + const selectedDatabases = await promptForDatabases( + updates.map((update) => update.database), + { + title: "Select databases to update", + }, + ); + if (selectedDatabases.length === 0) { + return; + } + + await Promise.all( + selectedDatabases.map((database) => { + const update = updates.find((update) => update.database === database); + if (!update) { + return; + } + + 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(), + ); + if (newDatabase === undefined) { + return; + } + + await databaseManager.removeDatabaseItem(update.databaseItem); + + await commandManager.execute("codeQLDatabases.focus"); + void window.showInformationMessage( + `Updated ${getLanguageDisplayName( + database.language, + )} database from GitHub.`, + ); + }, + { + title: `Updating ${getLanguageDisplayName( + database.language, + )} database from GitHub`, + }, + ); + }), + ); +} diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-database-updates.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-database-updates.test.ts new file mode 100644 index 00000000000..18670059654 --- /dev/null +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-database-updates.test.ts @@ -0,0 +1,557 @@ +import { faker } from "@faker-js/faker"; +import { Octokit } from "@octokit/rest"; +import { QuickPickItem, window } from "vscode"; +import { + mockDatabaseItem, + mockedObject, + mockedQuickPickItem, +} from "../../utils/mocking.helpers"; +import { CodeqlDatabase } from "../../../../src/databases/github-database-api"; +import { DatabaseManager } from "../../../../src/databases/local-databases"; +import { GitHubDatabaseConfig } from "../../../../src/config"; +import { CodeQLCliServer } from "../../../../src/codeql-cli/cli"; +import { createMockCommandManager } from "../../../__mocks__/commandsMock"; +import * as databaseFetcher from "../../../../src/databases/database-fetcher"; +import * as dialog from "../../../../src/common/vscode/dialog"; +import { + DatabaseUpdate, + askForGitHubDatabaseUpdate, + downloadDatabaseUpdateFromGitHub, + isNewerDatabaseAvailable, +} from "../../../../src/databases/github-database-updates"; + +describe("isNewerDatabaseAvailable", () => { + const owner = "github"; + const repo = "codeql"; + let databases: CodeqlDatabase[]; + let databaseManager: DatabaseManager; + + describe("when there are updates", () => { + beforeEach(() => { + databases = [ + mockedObject({ + language: "java", + commit_oid: "58e7476df3e464a0c9742b14cd4ca274b0993ebb", + }), + mockedObject({ + language: "swift", + commit_oid: "b81c25c0b73dd3c242068e8ab38bef25563a7c2d", + }), + mockedObject({ + language: "javascript", + commit_oid: "6e93915ff37ff8bcfc552d48f118895d60d0e7cd", + }), + mockedObject({ + language: "ql", + commit_oid: "9448fbfb88cdefe4298cc2e234a5a3c98958cae8", + }), + ]; + + databaseManager = mockedObject({ + databaseItems: [ + mockDatabaseItem({ + dateAdded: 1600477451789, + language: "java", + origin: { + type: "github", + repository: "github/codeql", + commitOid: "4487d1da9665231d1a076c60a78523f6275ad70f", + }, + }), + mockDatabaseItem({ + dateAdded: 50, + language: "swift", + origin: { + type: "github", + repository: "github/codeql", + commitOid: "2b020927d3c6eb407223a1baa3d6ce3597a3f88d", + }, + }), + mockDatabaseItem({ + dateAdded: 1700477451789, + language: "java", + origin: { + type: "github", + repository: "github/codeql", + commitOid: "17663af4e84a3a010fcb3f09cc06049797dfb22a", + }, + }), + mockDatabaseItem({ + dateAdded: faker.date.past().getTime(), + language: "golang", + origin: { + type: "github", + repository: "github/codeql", + }, + }), + mockDatabaseItem({ + dateAdded: faker.date.past().getTime(), + language: "ql", + origin: { + type: "github", + repository: "github/codeql", + }, + }), + mockDatabaseItem({ + language: "javascript", + origin: { + type: "github", + repository: "github/vscode-codeql", + commitOid: "fb360f9c09ac8c5edb2f18be5de4e80ea4c430d0", + }, + }), + ], + }); + }); + + it("returns the correct status", async () => { + expect( + isNewerDatabaseAvailable(databases, owner, repo, databaseManager), + ).toEqual({ + type: "updateAvailable", + databaseUpdates: [ + { + database: databases[0], + databaseItem: databaseManager.databaseItems[2], + }, + { + database: databases[3], + databaseItem: databaseManager.databaseItems[4], + }, + { + database: databases[1], + databaseItem: databaseManager.databaseItems[1], + }, + ], + }); + }); + }); + + describe("when there are no updates", () => { + beforeEach(() => { + databases = [ + mockedObject({ + language: "java", + commit_oid: "17663af4e84a3a010fcb3f09cc06049797dfb22a", + }), + ]; + + databaseManager = mockedObject({ + databaseItems: [ + mockDatabaseItem({ + dateAdded: 1700477451789, + language: "java", + origin: { + type: "github", + repository: "github/codeql", + commitOid: "17663af4e84a3a010fcb3f09cc06049797dfb22a", + }, + }), + mockDatabaseItem({ + dateAdded: 1600477451789, + language: "java", + origin: { + type: "github", + repository: "github/codeql", + commitOid: "4487d1da9665231d1a076c60a78523f6275ad70f", + }, + }), + ], + }); + }); + + it("returns the correct status", async () => { + expect( + isNewerDatabaseAvailable(databases, owner, repo, databaseManager), + ).toEqual({ + type: "upToDate", + }); + }); + }); + + describe("when there are no matching database items", () => { + beforeEach(() => { + databases = [ + mockedObject({ + language: "java", + commit_oid: "17663af4e84a3a010fcb3f09cc06049797dfb22a", + }), + ]; + + databaseManager = mockedObject({ + databaseItems: [ + mockDatabaseItem({ + language: "javascript", + origin: { + type: "github", + repository: "github/vscode-codeql", + commitOid: "fb360f9c09ac8c5edb2f18be5de4e80ea4c430d0", + }, + }), + ], + }); + }); + + it("returns the correct status", async () => { + expect( + isNewerDatabaseAvailable(databases, owner, repo, databaseManager), + ).toEqual({ + type: "noDatabase", + }); + }); + }); +}); + +describe("askForGitHubDatabaseUpdate", () => { + const setUpdate = jest.fn(); + let config: GitHubDatabaseConfig; + + const updates: DatabaseUpdate[] = [ + { + database: mockedObject({ + id: faker.number.int(), + created_at: faker.date.past().toISOString(), + commit_oid: faker.git.commitSha(), + language: "swift", + size: 27389673, + url: faker.internet.url({ + protocol: "https", + }), + }), + databaseItem: mockDatabaseItem(), + }, + ]; + + let showNeverAskAgainDialogSpy: jest.SpiedFunction< + typeof dialog.showNeverAskAgainDialog + >; + + beforeEach(() => { + config = mockedObject({ + setUpdate, + }); + + showNeverAskAgainDialogSpy = jest.spyOn(dialog, "showNeverAskAgainDialog"); + }); + + describe("when answering download to prompt", () => { + beforeEach(() => { + showNeverAskAgainDialogSpy.mockResolvedValue("Download"); + }); + + it("returns false", async () => { + expect(await askForGitHubDatabaseUpdate(updates, config)).toEqual(true); + + expect(setUpdate).not.toHaveBeenCalled(); + }); + }); + + describe("when answering not now to prompt", () => { + beforeEach(() => { + showNeverAskAgainDialogSpy.mockResolvedValue("Not now"); + }); + + it("returns false", async () => { + expect(await askForGitHubDatabaseUpdate(updates, config)).toEqual(false); + + expect(setUpdate).not.toHaveBeenCalled(); + }); + }); + + describe("when cancelling prompt", () => { + beforeEach(() => { + showNeverAskAgainDialogSpy.mockResolvedValue(undefined); + }); + + it("returns false", async () => { + expect(await askForGitHubDatabaseUpdate(updates, config)).toEqual(false); + + expect(setUpdate).not.toHaveBeenCalled(); + }); + }); + + describe("when answering never to prompt", () => { + beforeEach(() => { + showNeverAskAgainDialogSpy.mockResolvedValue("Never"); + }); + + it("returns false", async () => { + expect(await askForGitHubDatabaseUpdate(updates, config)).toEqual(false); + }); + + it("sets the config to never", async () => { + await askForGitHubDatabaseUpdate(updates, config); + + expect(setUpdate).toHaveBeenCalledWith("never"); + }); + }); +}); + +describe("downloadDatabaseUpdateFromGitHub", () => { + let octokit: Octokit; + const owner = "github"; + const repo = "codeql"; + let databaseManager: DatabaseManager; + const storagePath = "/a/b/c/d"; + let cliServer: CodeQLCliServer; + const commandManager = createMockCommandManager(); + + let updates: DatabaseUpdate[] = [ + { + database: mockedObject({ + id: faker.number.int(), + created_at: faker.date.past().toISOString(), + commit_oid: faker.git.commitSha(), + language: "swift", + size: 27389673, + url: faker.internet.url({ + protocol: "https", + }), + }), + databaseItem: mockDatabaseItem({ + hasSourceArchiveInExplorer: () => false, + }), + }, + ]; + + let showQuickPickSpy: jest.SpiedFunction; + let downloadGitHubDatabaseFromUrlSpy: jest.SpiedFunction< + typeof databaseFetcher.downloadGitHubDatabaseFromUrl + >; + + beforeEach(() => { + octokit = mockedObject({}); + databaseManager = mockedObject({ + currentDatabaseItem: mockDatabaseItem(), + }); + cliServer = mockedObject({}); + + showQuickPickSpy = jest.spyOn(window, "showQuickPick").mockResolvedValue( + mockedQuickPickItem([ + mockedObject({ + database: updates[0].database, + }), + ]), + ); + downloadGitHubDatabaseFromUrlSpy = jest + .spyOn(databaseFetcher, "downloadGitHubDatabaseFromUrl") + .mockResolvedValue(undefined); + }); + + it("downloads the database", async () => { + await downloadDatabaseUpdateFromGitHub( + octokit, + owner, + repo, + updates, + databaseManager, + storagePath, + cliServer, + commandManager, + ); + + expect(downloadGitHubDatabaseFromUrlSpy).toHaveBeenCalledTimes(1); + expect(downloadGitHubDatabaseFromUrlSpy).toHaveBeenCalledWith( + updates[0].database.url, + updates[0].database.id, + updates[0].database.created_at, + updates[0].database.commit_oid, + owner, + repo, + octokit, + expect.anything(), + databaseManager, + storagePath, + cliServer, + false, + false, + ); + expect(showQuickPickSpy).not.toHaveBeenCalled(); + }); + + describe("when there are multiple languages", () => { + beforeEach(() => { + updates = [ + { + database: mockedObject({ + id: faker.number.int(), + created_at: faker.date.past().toISOString(), + commit_oid: faker.git.commitSha(), + language: "swift", + size: 27389673, + url: faker.internet.url({ + protocol: "https", + }), + }), + databaseItem: mockDatabaseItem({ + hasSourceArchiveInExplorer: () => false, + }), + }, + { + database: mockedObject({ + id: faker.number.int(), + created_at: faker.date.past().toISOString(), + commit_oid: null, + language: "go", + size: 2930572385, + url: faker.internet.url({ + protocol: "https", + }), + }), + databaseItem: mockDatabaseItem({ + hasSourceArchiveInExplorer: () => true, + }), + }, + ]; + + databaseManager = mockedObject({ + currentDatabaseItem: updates[1].databaseItem, + }); + }); + + it("downloads a single selected language", async () => { + showQuickPickSpy.mockResolvedValue( + mockedQuickPickItem([ + mockedObject({ + database: updates[1].database, + }), + ]), + ); + + await downloadDatabaseUpdateFromGitHub( + octokit, + owner, + repo, + updates, + databaseManager, + storagePath, + cliServer, + commandManager, + ); + + expect(downloadGitHubDatabaseFromUrlSpy).toHaveBeenCalledTimes(1); + expect(downloadGitHubDatabaseFromUrlSpy).toHaveBeenCalledWith( + updates[1].database.url, + updates[1].database.id, + updates[1].database.created_at, + updates[1].database.commit_oid, + owner, + repo, + octokit, + expect.anything(), + databaseManager, + storagePath, + cliServer, + true, + true, + ); + expect(showQuickPickSpy).toHaveBeenCalledWith( + [ + expect.objectContaining({ + label: "Go", + description: "2794.8 MB", + database: updates[1].database, + }), + expect.objectContaining({ + label: "Swift", + description: "26.1 MB", + database: updates[0].database, + }), + ], + expect.anything(), + ); + }); + + it("downloads multiple selected languages", async () => { + showQuickPickSpy.mockResolvedValue( + mockedQuickPickItem([ + mockedObject({ + database: updates[0].database, + }), + mockedObject({ + database: updates[1].database, + }), + ]), + ); + + await downloadDatabaseUpdateFromGitHub( + octokit, + owner, + repo, + updates, + databaseManager, + storagePath, + cliServer, + commandManager, + ); + + expect(downloadGitHubDatabaseFromUrlSpy).toHaveBeenCalledTimes(2); + expect(downloadGitHubDatabaseFromUrlSpy).toHaveBeenCalledWith( + updates[0].database.url, + updates[0].database.id, + updates[0].database.created_at, + updates[0].database.commit_oid, + owner, + repo, + octokit, + expect.anything(), + databaseManager, + storagePath, + cliServer, + false, + false, + ); + expect(downloadGitHubDatabaseFromUrlSpy).toHaveBeenCalledWith( + updates[1].database.url, + updates[1].database.id, + updates[1].database.created_at, + updates[1].database.commit_oid, + owner, + repo, + octokit, + expect.anything(), + databaseManager, + storagePath, + cliServer, + true, + true, + ); + expect(showQuickPickSpy).toHaveBeenCalledWith( + [ + expect.objectContaining({ + label: "Go", + description: "2794.8 MB", + database: updates[1].database, + }), + expect.objectContaining({ + label: "Swift", + description: "26.1 MB", + database: updates[0].database, + }), + ], + expect.anything(), + ); + }); + + describe("when not selecting language", () => { + beforeEach(() => { + showQuickPickSpy.mockResolvedValue(undefined); + }); + + it("does not download the database", async () => { + await downloadDatabaseUpdateFromGitHub( + octokit, + owner, + repo, + updates, + databaseManager, + storagePath, + cliServer, + commandManager, + ); + + expect(downloadGitHubDatabaseFromUrlSpy).not.toHaveBeenCalled(); + }); + }); + }); +}); From 7b3a55e2bf83a4bee5cb19b4bc2d62a60b04f614 Mon Sep 17 00:00:00 2001 From: Koen Vlaswinkel Date: Thu, 23 Nov 2023 10:11:36 +0100 Subject: [PATCH 2/5] Refactor handling of updateStatus --- .../src/databases/github-database-module.ts | 97 ++++++++++++------- 1 file changed, 63 insertions(+), 34 deletions(-) diff --git a/extensions/ql-vscode/src/databases/github-database-module.ts b/extensions/ql-vscode/src/databases/github-database-module.ts index beab4fc4c8e..7c18011a53b 100644 --- a/extensions/ql-vscode/src/databases/github-database-module.ts +++ b/extensions/ql-vscode/src/databases/github-database-module.ts @@ -11,12 +11,18 @@ import { import { GitHubDatabaseConfig, GitHubDatabaseConfigListener } from "../config"; import { DatabaseManager } from "./local-databases"; import { CodeQLCliServer } from "../codeql-cli/cli"; -import { listDatabases, ListDatabasesResult } from "./github-database-api"; +import { + CodeqlDatabase, + listDatabases, + ListDatabasesResult, +} from "./github-database-api"; import { askForGitHubDatabaseUpdate, + DatabaseUpdate, downloadDatabaseUpdateFromGitHub, isNewerDatabaseAvailable, } from "./github-database-updates"; +import { Octokit } from "@octokit/rest"; export class GithubDatabaseModule extends DisposableObject { private readonly config: GitHubDatabaseConfig; @@ -131,42 +137,39 @@ export class GithubDatabaseModule extends DisposableObject { githubRepository.name, this.databaseManager, ); - if (updateStatus.type === "upToDate") { - return; - } - if (updateStatus.type === "updateAvailable") { - if (this.config.update === "never") { + switch (updateStatus.type) { + case "upToDate": return; - } - - if ( - !(await askForGitHubDatabaseUpdate( + case "updateAvailable": + await this.updateGitHubDatabase( + octokit, + githubRepository.owner, + githubRepository.name, updateStatus.databaseUpdates, - this.config, - )) - ) { - return; - } - - await downloadDatabaseUpdateFromGitHub( - octokit, - githubRepository.owner, - githubRepository.name, - updateStatus.databaseUpdates, - this.databaseManager, - this.databaseStoragePath, - this.cliServer, - this.app.commands, - ); - - return; - } - - if (updateStatus.type !== "noDatabase") { - assertNever(updateStatus); + ); + break; + case "noDatabase": + await this.downloadGitHubDatabase( + octokit, + githubRepository.owner, + githubRepository.name, + databases, + promptedForCredentials, + ); + break; + default: + assertNever(updateStatus); } + } + private async downloadGitHubDatabase( + octokit: Octokit, + owner: string, + repo: string, + databases: CodeqlDatabase[], + promptedForCredentials: boolean, + ) { // If the user already had an access token, first ask if they even want to download the DB. if (!promptedForCredentials) { if (!(await askForGitHubDatabaseDownload(databases, this.config))) { @@ -176,8 +179,8 @@ export class GithubDatabaseModule extends DisposableObject { await downloadDatabaseFromGitHub( octokit, - githubRepository.owner, - githubRepository.name, + owner, + repo, databases, this.databaseManager, this.databaseStoragePath, @@ -185,4 +188,30 @@ export class GithubDatabaseModule extends DisposableObject { this.app.commands, ); } + + private async updateGitHubDatabase( + octokit: Octokit, + owner: string, + repo: string, + databaseUpdates: DatabaseUpdate[], + ): Promise { + if (this.config.update === "never") { + return; + } + + if (!(await askForGitHubDatabaseUpdate(databaseUpdates, this.config))) { + return; + } + + await downloadDatabaseUpdateFromGitHub( + octokit, + owner, + repo, + databaseUpdates, + this.databaseManager, + this.databaseStoragePath, + this.cliServer, + this.app.commands, + ); + } } From 5d1b2926ccbe51482ad2003871a298f165397177 Mon Sep 17 00:00:00 2001 From: Koen Vlaswinkel Date: Thu, 23 Nov 2023 10:14:04 +0100 Subject: [PATCH 3/5] Add comment about update check --- extensions/ql-vscode/src/databases/github-database-updates.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/extensions/ql-vscode/src/databases/github-database-updates.ts b/extensions/ql-vscode/src/databases/github-database-updates.ts index 12ad48186f5..2cf6ed006fd 100644 --- a/extensions/ql-vscode/src/databases/github-database-updates.ts +++ b/extensions/ql-vscode/src/databases/github-database-updates.ts @@ -78,6 +78,10 @@ export function isNewerDatabaseAvailable( return null; } + // We only consider databases to be updated if they have a different `commit_oid` than the + // one we have stored. If they have the same `commit_oid`, then they are the same database. + // This means that older databases which do not have a `commit_oid` (`null`) are always + // considered up-to-date. if (matchingDatabase.commit_oid === origin.commitOid) { return null; } From e66d76aca8b8703a6cc9c52f2519e914b5ea060f Mon Sep 17 00:00:00 2001 From: Koen Vlaswinkel Date: Thu, 23 Nov 2023 10:17:07 +0100 Subject: [PATCH 4/5] Consider created_at for database updates --- .../src/databases/github-database-updates.ts | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/extensions/ql-vscode/src/databases/github-database-updates.ts b/extensions/ql-vscode/src/databases/github-database-updates.ts index 2cf6ed006fd..9217a171a8d 100644 --- a/extensions/ql-vscode/src/databases/github-database-updates.ts +++ b/extensions/ql-vscode/src/databases/github-database-updates.ts @@ -34,6 +34,11 @@ type DatabaseUpdateStatus = | DatabaseUpdateStatusUpToDate | DatabaseUpdateStatusNoDatabase; +/** + * Check whether a newer database is available for the given repository. Databases are considered updated if: + * - They have a different commit OID, or + * - They have the same commit OID, but the remote database was created after the local database. + */ export function isNewerDatabaseAvailable( databases: CodeqlDatabase[], owner: string, @@ -78,12 +83,16 @@ export function isNewerDatabaseAvailable( return null; } - // We only consider databases to be updated if they have a different `commit_oid` than the - // one we have stored. If they have the same `commit_oid`, then they are the same database. - // This means that older databases which do not have a `commit_oid` (`null`) are always - // considered up-to-date. + // If they are not equal, we assume that the remote database is newer. if (matchingDatabase.commit_oid === origin.commitOid) { - return null; + const remoteDatabaseCreatedAt = new Date(matchingDatabase.created_at); + const localDatabaseCreatedAt = new Date(origin.databaseCreatedAt); + + // If the remote database was created before the local database, + // we assume that the local database is newer. + if (remoteDatabaseCreatedAt <= localDatabaseCreatedAt) { + return null; + } } return { From 4f988de36ddba3ebf1f24522b8af885759965a29 Mon Sep 17 00:00:00 2001 From: Koen Vlaswinkel Date: Thu, 23 Nov 2023 10:27:28 +0100 Subject: [PATCH 5/5] Update tests for `isNewerDatabaseAvailable` --- .../databases/github-database-updates.test.ts | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-database-updates.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-database-updates.test.ts index 18670059654..25be578f052 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-database-updates.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/github-database-updates.test.ts @@ -32,18 +32,32 @@ describe("isNewerDatabaseAvailable", () => { mockedObject({ language: "java", commit_oid: "58e7476df3e464a0c9742b14cd4ca274b0993ebb", + created_at: "2023-11-22T09:20:59.185Z", }), mockedObject({ language: "swift", commit_oid: "b81c25c0b73dd3c242068e8ab38bef25563a7c2d", + created_at: "2023-11-22T09:21:53.257Z", }), mockedObject({ language: "javascript", commit_oid: "6e93915ff37ff8bcfc552d48f118895d60d0e7cd", + created_at: "2023-11-20T09:20:52.185Z", }), mockedObject({ language: "ql", commit_oid: "9448fbfb88cdefe4298cc2e234a5a3c98958cae8", + created_at: "2023-11-21T09:20:59.185Z", + }), + mockedObject({ + language: "ruby", + commit_oid: "30220ebe8a36a22c4b6200fd207476d03717be4c", + created_at: "2023-11-23T09:20:59.185Z", + }), + mockedObject({ + language: "csharp", + commit_oid: "1dad8b67751834ea61344effacf3ac8a88929289", + created_at: "2023-11-20T09:20:59.185Z", }), ]; @@ -56,6 +70,7 @@ describe("isNewerDatabaseAvailable", () => { type: "github", repository: "github/codeql", commitOid: "4487d1da9665231d1a076c60a78523f6275ad70f", + databaseCreatedAt: "2023-10-22T09:20:59.185Z", }, }), mockDatabaseItem({ @@ -65,6 +80,7 @@ describe("isNewerDatabaseAvailable", () => { type: "github", repository: "github/codeql", commitOid: "2b020927d3c6eb407223a1baa3d6ce3597a3f88d", + databaseCreatedAt: "2023-10-22T09:20:59.185Z", }, }), mockDatabaseItem({ @@ -74,6 +90,7 @@ describe("isNewerDatabaseAvailable", () => { type: "github", repository: "github/codeql", commitOid: "17663af4e84a3a010fcb3f09cc06049797dfb22a", + databaseCreatedAt: "2023-10-22T09:20:59.185Z", }, }), mockDatabaseItem({ @@ -82,6 +99,7 @@ describe("isNewerDatabaseAvailable", () => { origin: { type: "github", repository: "github/codeql", + databaseCreatedAt: "2023-10-22T09:19:59.185Z", }, }), mockDatabaseItem({ @@ -90,6 +108,7 @@ describe("isNewerDatabaseAvailable", () => { origin: { type: "github", repository: "github/codeql", + databaseCreatedAt: "2023-10-20T09:20:59.185Z", }, }), mockDatabaseItem({ @@ -98,6 +117,27 @@ describe("isNewerDatabaseAvailable", () => { type: "github", repository: "github/vscode-codeql", commitOid: "fb360f9c09ac8c5edb2f18be5de4e80ea4c430d0", + databaseCreatedAt: "2023-10-22T09:20:59.185Z", + }, + }), + mockDatabaseItem({ + dateAdded: faker.date.past().getTime(), + language: "ruby", + origin: { + type: "github", + repository: "github/codeql", + commitOid: "30220ebe8a36a22c4b6200fd207476d03717be4c", + databaseCreatedAt: "2023-10-22T09:20:59.185Z", + }, + }), + mockDatabaseItem({ + dateAdded: faker.date.past().getTime(), + language: "csharp", + origin: { + type: "github", + repository: "github/codeql", + commitOid: "1dad8b67751834ea61344effacf3ac8a88929289", + databaseCreatedAt: "2023-11-23T09:20:59.185Z", }, }), ], @@ -110,18 +150,26 @@ describe("isNewerDatabaseAvailable", () => { ).toEqual({ type: "updateAvailable", databaseUpdates: [ + // java: different commit_oid, last dateAdded { database: databases[0], databaseItem: databaseManager.databaseItems[2], }, { + // ql: commit_oid on remote, not on local database: databases[3], databaseItem: databaseManager.databaseItems[4], }, { + // swift: different commit_oid database: databases[1], databaseItem: databaseManager.databaseItems[1], }, + { + // ruby: same commit_oid, newer created_at + database: databases[4], + databaseItem: databaseManager.databaseItems[6], + }, ], }); }); @@ -133,6 +181,7 @@ describe("isNewerDatabaseAvailable", () => { mockedObject({ language: "java", commit_oid: "17663af4e84a3a010fcb3f09cc06049797dfb22a", + created_at: "2023-11-22T09:21:53.257Z", }), ]; @@ -145,6 +194,7 @@ describe("isNewerDatabaseAvailable", () => { type: "github", repository: "github/codeql", commitOid: "17663af4e84a3a010fcb3f09cc06049797dfb22a", + databaseCreatedAt: "2023-11-22T09:21:53.257Z", }, }), mockDatabaseItem({ @@ -154,6 +204,7 @@ describe("isNewerDatabaseAvailable", () => { type: "github", repository: "github/codeql", commitOid: "4487d1da9665231d1a076c60a78523f6275ad70f", + databaseCreatedAt: "2023-10-22T09:20:59.185Z", }, }), ],