diff --git a/extensions/ql-vscode/src/databases/local-databases/database-item-impl.ts b/extensions/ql-vscode/src/databases/local-databases/database-item-impl.ts index bd2c76786d8..16f18432466 100644 --- a/extensions/ql-vscode/src/databases/local-databases/database-item-impl.ts +++ b/extensions/ql-vscode/src/databases/local-databases/database-item-impl.ts @@ -3,7 +3,6 @@ import * as cli from "../../codeql-cli/cli"; import vscode from "vscode"; import { FullDatabaseOptions } from "./database-options"; import { basename, dirname, join, relative } from "path"; -import { asError } from "../../pure/helpers-pure"; import { decodeSourceArchiveUri, encodeArchiveBasePath, @@ -15,12 +14,11 @@ import { isLikelyDatabaseRoot } from "../../helpers"; import { stat } from "fs-extra"; import { pathsEqual } from "../../pure/files"; import { DatabaseContents } from "./database-contents"; -import { DatabaseResolver } from "./database-resolver"; -import { DatabaseChangedEvent, DatabaseEventKind } from "./database-events"; export class DatabaseItemImpl implements DatabaseItem { - private _error: Error | undefined = undefined; - private _contents: DatabaseContents | undefined; + // These are only public in the implementation, they are readonly in the interface + public error: Error | undefined = undefined; + public contents: DatabaseContents | undefined; /** A cache of database info */ private _dbinfo: cli.DbInfo | undefined; @@ -28,16 +26,15 @@ export class DatabaseItemImpl implements DatabaseItem { public readonly databaseUri: vscode.Uri, contents: DatabaseContents | undefined, private options: FullDatabaseOptions, - private readonly onChanged: (event: DatabaseChangedEvent) => void, ) { - this._contents = contents; + this.contents = contents; } public get name(): string { if (this.options.displayName) { return this.options.displayName; - } else if (this._contents) { - return this._contents.name; + } else if (this.contents) { + return this.contents.name; } else { return basename(this.databaseUri.fsPath); } @@ -48,45 +45,17 @@ export class DatabaseItemImpl implements DatabaseItem { } public get sourceArchive(): vscode.Uri | undefined { - if (this.options.ignoreSourceArchive || this._contents === undefined) { + if (this.options.ignoreSourceArchive || this.contents === undefined) { return undefined; } else { - return this._contents.sourceArchiveUri; + return this.contents.sourceArchiveUri; } } - public get contents(): DatabaseContents | undefined { - return this._contents; - } - public get dateAdded(): number | undefined { return this.options.dateAdded; } - public get error(): Error | undefined { - return this._error; - } - - public async refresh(): Promise { - try { - try { - this._contents = await DatabaseResolver.resolveDatabaseContents( - this.databaseUri, - ); - this._error = undefined; - } catch (e) { - this._contents = undefined; - this._error = asError(e); - throw e; - } - } finally { - this.onChanged({ - kind: DatabaseEventKind.Refresh, - item: this, - }); - } - } - public resolveSourceFile(uriStr: string | undefined): vscode.Uri { const sourceArchive = this.sourceArchive; const uri = uriStr ? vscode.Uri.parse(uriStr, true) : undefined; diff --git a/extensions/ql-vscode/src/databases/local-databases/database-item.ts b/extensions/ql-vscode/src/databases/local-databases/database-item.ts index 0da295afbbc..1794d8e753d 100644 --- a/extensions/ql-vscode/src/databases/local-databases/database-item.ts +++ b/extensions/ql-vscode/src/databases/local-databases/database-item.ts @@ -27,15 +27,7 @@ export interface DatabaseItem { /** If the database is invalid, describes why. */ readonly error: Error | undefined; - /** - * Resolves the contents of the database. - * - * @remarks - * The contents include the database directory, source archive, and metadata about the database. - * If the database is invalid, `this.error` is updated with the error object that describes why - * the database is invalid. This error is also thrown. - */ - refresh(): Promise; + /** * Resolves a filename to its URI in the source archive. * 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 1383b179e11..726ab554a68 100644 --- a/extensions/ql-vscode/src/databases/local-databases/database-manager.ts +++ b/extensions/ql-vscode/src/databases/local-databases/database-manager.ts @@ -83,7 +83,7 @@ export class DatabaseManager extends DisposableObject { readonly onDidChangeCurrentDatabaseItem = this._onDidChangeCurrentDatabaseItem.event; - private readonly _databaseItems: DatabaseItem[] = []; + private readonly _databaseItems: DatabaseItemImpl[] = []; private _currentDatabaseItem: DatabaseItem | undefined = undefined; constructor( @@ -127,8 +127,8 @@ export class DatabaseManager extends DisposableObject { * * Typically, the item will have been created by {@link createOrOpenDatabaseItem} or {@link openDatabase}. */ - public async addExistingDatabaseItem( - databaseItem: DatabaseItem, + private async addExistingDatabaseItem( + databaseItem: DatabaseItemImpl, progress: ProgressCallback, makeSelected: boolean, token: vscode.CancellationToken, @@ -162,7 +162,7 @@ export class DatabaseManager extends DisposableObject { private async createDatabaseItem( uri: vscode.Uri, displayName: string | undefined, - ): Promise { + ): Promise { const contents = await DatabaseResolver.resolveDatabaseContents(uri); // Ignore the source archive for QLTest databases by default. const isQLTestDatabase = extname(uri.fsPath) === ".testproj"; @@ -173,14 +173,7 @@ export class DatabaseManager extends DisposableObject { dateAdded: Date.now(), language: await this.getPrimaryLanguage(uri.fsPath), }; - const databaseItem = new DatabaseItemImpl( - uri, - contents, - fullOptions, - (event) => { - this._onDidChangeDatabaseItem.fire(event); - }, - ); + const databaseItem = new DatabaseItemImpl(uri, contents, fullOptions); return databaseItem; } @@ -329,7 +322,7 @@ export class DatabaseManager extends DisposableObject { progress: ProgressCallback, token: vscode.CancellationToken, state: PersistedDatabaseItem, - ): Promise { + ): Promise { let displayName: string | undefined = undefined; let ignoreSourceArchive = false; let dateAdded = undefined; @@ -359,14 +352,7 @@ export class DatabaseManager extends DisposableObject { dateAdded, language, }; - const item = new DatabaseItemImpl( - dbBaseUri, - undefined, - fullOptions, - (event) => { - this._onDidChangeDatabaseItem.fire(event); - }, - ); + const item = new DatabaseItemImpl(dbBaseUri, undefined, fullOptions); // Avoid persisting the database state after adding since that should happen only after // all databases have been added. @@ -407,7 +393,7 @@ export class DatabaseManager extends DisposableObject { database, ); try { - await databaseItem.refresh(); + await this.refreshDatabase(databaseItem); await this.registerDatabase(progress, token, databaseItem); if (currentDatabaseUri === database.uri) { await this.setCurrentDatabaseItem(databaseItem, true); @@ -449,8 +435,12 @@ export class DatabaseManager extends DisposableObject { item: DatabaseItem | undefined, skipRefresh = false, ): Promise { - if (!skipRefresh && item !== undefined) { - await item.refresh(); // Will throw on invalid database. + if ( + !skipRefresh && + item !== undefined && + item instanceof DatabaseItemImpl + ) { + await this.refreshDatabase(item); // Will throw on invalid database. } if (this._currentDatabaseItem !== item) { this._currentDatabaseItem = item; @@ -499,7 +489,7 @@ export class DatabaseManager extends DisposableObject { private async addDatabaseItem( progress: ProgressCallback, token: vscode.CancellationToken, - item: DatabaseItem, + item: DatabaseItemImpl, updatePersistedState = true, ) { this._databaseItems.push(item); @@ -616,6 +606,34 @@ export class DatabaseManager extends DisposableObject { await this.qs.registerDatabase(progress, token, dbItem); } + /** + * Resolves the contents of the database. + * + * @remarks + * The contents include the database directory, source archive, and metadata about the database. + * If the database is invalid, `databaseItem.error` is updated with the error object that describes why + * the database is invalid. This error is also thrown. + */ + private async refreshDatabase(databaseItem: DatabaseItemImpl) { + try { + try { + databaseItem.contents = await DatabaseResolver.resolveDatabaseContents( + databaseItem.databaseUri, + ); + databaseItem.error = undefined; + } catch (e) { + databaseItem.contents = undefined; + databaseItem.error = asError(e); + throw e; + } + } finally { + this._onDidChangeDatabaseItem.fire({ + kind: DatabaseEventKind.Refresh, + item: databaseItem, + }); + } + } + private updatePersistedCurrentDatabaseItem(): void { void this.ctx.workspaceState.update( CURRENT_DB, diff --git a/extensions/ql-vscode/test/factories/databases/databases.ts b/extensions/ql-vscode/test/factories/databases/databases.ts index f6aa2aebf94..46826917af9 100644 --- a/extensions/ql-vscode/test/factories/databases/databases.ts +++ b/extensions/ql-vscode/test/factories/databases/databases.ts @@ -33,7 +33,6 @@ export function createMockDB( datasetUri: databaseUri, } as DatabaseContents, dbOptions, - () => void 0, ); } diff --git a/extensions/ql-vscode/test/vscode-tests/cli-integration/skeleton-query-wizard.test.ts b/extensions/ql-vscode/test/vscode-tests/cli-integration/skeleton-query-wizard.test.ts index a50a08c151d..949d7958023 100644 --- a/extensions/ql-vscode/test/vscode-tests/cli-integration/skeleton-query-wizard.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/cli-integration/skeleton-query-wizard.test.ts @@ -546,9 +546,7 @@ describe("SkeletonQueryWizard", () => { dateAdded: 123, } as FullDatabaseOptions); - jest - .spyOn(mockDbItem, "error", "get") - .mockReturnValue(asError("database go boom!")); + mockDbItem.error = asError("database go boom!"); const sortedList = await SkeletonQueryWizard.sortDatabaseItemsByDateAdded([ diff --git a/extensions/ql-vscode/test/vscode-tests/minimal-workspace/local-databases.test.ts b/extensions/ql-vscode/test/vscode-tests/minimal-workspace/local-databases.test.ts index 00bad35c242..a05418c390c 100644 --- a/extensions/ql-vscode/test/vscode-tests/minimal-workspace/local-databases.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/minimal-workspace/local-databases.test.ts @@ -327,7 +327,7 @@ describe("local databases", () => { mockDbOptions(), Uri.parse("file:/sourceArchive-uri/"), ); - (db as any)._contents.sourceArchiveUri = undefined; + (db as any).contents.sourceArchiveUri = undefined; expect(() => db.resolveSourceFile("abc")).toThrowError( "Scheme is missing", ); @@ -339,7 +339,7 @@ describe("local databases", () => { mockDbOptions(), Uri.parse("file:/sourceArchive-uri/"), ); - (db as any)._contents.sourceArchiveUri = undefined; + (db as any).contents.sourceArchiveUri = undefined; expect(() => db.resolveSourceFile("http://abc")).toThrowError( "Invalid uri scheme", ); @@ -352,7 +352,7 @@ describe("local databases", () => { mockDbOptions(), Uri.parse("file:/sourceArchive-uri/"), ); - (db as any)._contents.sourceArchiveUri = undefined; + (db as any).contents.sourceArchiveUri = undefined; const resolved = db.resolveSourceFile(undefined); expect(resolved.toString(true)).toBe(dbLocationUri(dir).toString(true)); }); @@ -363,7 +363,7 @@ describe("local databases", () => { mockDbOptions(), Uri.parse("file:/sourceArchive-uri/"), ); - (db as any)._contents.sourceArchiveUri = undefined; + (db as any).contents.sourceArchiveUri = undefined; const resolved = db.resolveSourceFile("file:"); expect(resolved.toString()).toBe("file:///"); }); diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/query-testing/test-runner.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/query-testing/test-runner.test.ts index 628355fb708..f4d9a3902ea 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/query-testing/test-runner.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/query-testing/test-runner.test.ts @@ -40,17 +40,11 @@ describe("test-runner", () => { Uri.file("/path/to/test/dir/dir.testproj"), undefined, mockedObject({ displayName: "custom display name" }), - (_) => { - /* no change event listener */ - }, ); const postTestDatabaseItem = new DatabaseItemImpl( Uri.file("/path/to/test/dir/dir.testproj"), undefined, mockedObject({ displayName: "default name" }), - (_) => { - /* no change event listener */ - }, ); beforeEach(() => {