Extract DatabaseItem.refresh to DatabaseManager

This moves the `refresh` method from `DatabaseItem` to `DatabaseManager`
and makes it private. This makes the `DatabaseItem` interface smaller
and more focused and ensures that `refresh` cannot be called from
outside of the `DatabaseManager`.
This commit is contained in:
Koen Vlaswinkel
2023-05-25 15:03:25 +02:00
parent 295a08f85a
commit 09fc0f3040
7 changed files with 51 additions and 81 deletions

View File

@@ -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<void> {
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;

View File

@@ -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<void>;
/**
* Resolves a filename to its URI in the source archive.
*

View File

@@ -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;
}
@@ -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<void> {
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;
@@ -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,

View File

@@ -33,7 +33,6 @@ export function createMockDB(
datasetUri: databaseUri,
} as DatabaseContents,
dbOptions,
() => void 0,
);
}

View File

@@ -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([

View File

@@ -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:///");
});

View File

@@ -40,17 +40,11 @@ describe("test-runner", () => {
Uri.file("/path/to/test/dir/dir.testproj"),
undefined,
mockedObject<FullDatabaseOptions>({ displayName: "custom display name" }),
(_) => {
/* no change event listener */
},
);
const postTestDatabaseItem = new DatabaseItemImpl(
Uri.file("/path/to/test/dir/dir.testproj"),
undefined,
mockedObject<FullDatabaseOptions>({ displayName: "default name" }),
(_) => {
/* no change event listener */
},
);
beforeEach(() => {