Skip to content

Commit 6e41241

Browse files
committed
Ensure we're selecting database in single rooted workspace
After some testing of the wizard with a single rooted workspace (`github/code-scanning`) we discovered a general VSCode extension bug whereby after we download a database, we don't select it. This isn't an issue in the wizard, but it does affect us as it means we'll generate the QL pack, download the db for you but then you won't know that we haven't selected your database. So let's make sure our flow works for this case by explicitly selecting the database once it's downloaded. We've noticed by testing that we need to set the current database item before we call `addDatabaseSourceArchiveFolder()`. Once that method is called, the call to `setCurrentDatabaseItem()` is ignored. We've had to make some changes to the openDatabase() method to select a database item by default, since most places where we call `openDatabase()` also immediately select the item. There is one exception [1] in the test-runner.ts file, where we set the current database item under special conditions. For this reason, we've made the behaviour configurable and tried to add some descriptive naming to the params so that it's easy to understand what the config is doing. [1]: https://github.com/github/vscode-codeql/blob/4170e7f7a7026b3ebcc75b7509dd55f317293a4d/extensions/ql-vscode/src/test-runner.ts#L120-L124
1 parent 34d4fd4 commit 6e41241

File tree

4 files changed

+42
-6
lines changed

4 files changed

+42
-6
lines changed

extensions/ql-vscode/src/databaseFetcher.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -317,13 +317,15 @@ async function databaseArchiveFetcher(
317317
});
318318
await ensureZippedSourceLocation(dbPath);
319319

320+
const makeSelected = true;
321+
320322
const item = await databaseManager.openDatabase(
321323
progress,
322324
token,
323325
Uri.file(dbPath),
326+
makeSelected,
324327
nameOverride,
325328
);
326-
await databaseManager.setCurrentDatabaseItem(item);
327329
return item;
328330
} else {
329331
throw new Error("Database not found in archive.");

extensions/ql-vscode/src/local-databases-ui.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -307,17 +307,20 @@ export class DatabaseUI extends DisposableObject {
307307
);
308308

309309
let databaseItem = this.databaseManager.findDatabaseItem(uri);
310-
const isTutorialDatabase = true;
311310
if (databaseItem === undefined) {
311+
const makeSelected = true;
312+
const nameOverride = "CodeQL Tutorial Database";
313+
const isTutorialDatabase = true;
314+
312315
databaseItem = await this.databaseManager.openDatabase(
313316
progress,
314317
token,
315318
uri,
316-
"CodeQL Tutorial Database",
319+
makeSelected,
320+
nameOverride,
317321
isTutorialDatabase,
318322
);
319323
}
320-
await this.databaseManager.setCurrentDatabaseItem(databaseItem);
321324
await this.handleTourDependencies();
322325
}
323326
} catch (e) {
@@ -758,14 +761,17 @@ export class DatabaseUI extends DisposableObject {
758761
uri: Uri,
759762
): Promise<DatabaseItem | undefined> {
760763
let databaseItem = this.databaseManager.findDatabaseItem(uri);
764+
765+
const makeSelected = true;
766+
761767
if (databaseItem === undefined) {
762768
databaseItem = await this.databaseManager.openDatabase(
763769
progress,
764770
token,
765771
uri,
772+
makeSelected,
766773
);
767774
}
768-
await this.databaseManager.setCurrentDatabaseItem(databaseItem);
769775

770776
return databaseItem;
771777
}

extensions/ql-vscode/src/local-databases.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -621,6 +621,7 @@ export class DatabaseManager extends DisposableObject {
621621
progress: ProgressCallback,
622622
token: vscode.CancellationToken,
623623
uri: vscode.Uri,
624+
makeSelected = false,
624625
displayName?: string,
625626
isTutorialDatabase?: boolean,
626627
): Promise<DatabaseItem> {
@@ -629,6 +630,7 @@ export class DatabaseManager extends DisposableObject {
629630
return await this.addExistingDatabaseItem(
630631
databaseItem,
631632
progress,
633+
makeSelected,
632634
token,
633635
isTutorialDatabase,
634636
);
@@ -643,6 +645,7 @@ export class DatabaseManager extends DisposableObject {
643645
public async addExistingDatabaseItem(
644646
databaseItem: DatabaseItem,
645647
progress: ProgressCallback,
648+
makeSelected = true,
646649
token: vscode.CancellationToken,
647650
isTutorialDatabase?: boolean,
648651
): Promise<DatabaseItem> {
@@ -652,6 +655,9 @@ export class DatabaseManager extends DisposableObject {
652655
}
653656

654657
await this.addDatabaseItem(progress, token, databaseItem);
658+
if (makeSelected) {
659+
await this.setCurrentDatabaseItem(databaseItem);
660+
}
655661
await this.addDatabaseSourceArchiveFolder(databaseItem);
656662

657663
if (isCodespacesTemplate() && !isTutorialDatabase) {

extensions/ql-vscode/test/vscode-tests/minimal-workspace/local-databases.test.ts

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -708,6 +708,7 @@ describe("local databases", () => {
708708
describe("openDatabase", () => {
709709
let createSkeletonPacksSpy: jest.SpyInstance;
710710
let resolveDatabaseContentsSpy: jest.SpyInstance;
711+
let setCurrentDatabaseItemSpy: jest.SpyInstance;
711712
let addDatabaseSourceArchiveFolderSpy: jest.SpyInstance;
712713
let mockDbItem: DatabaseItemImpl;
713714

@@ -722,6 +723,11 @@ describe("local databases", () => {
722723
.spyOn(DatabaseResolver, "resolveDatabaseContents")
723724
.mockResolvedValue({} as DatabaseContentsWithDbScheme);
724725

726+
setCurrentDatabaseItemSpy = jest.spyOn(
727+
databaseManager,
728+
"setCurrentDatabaseItem",
729+
);
730+
725731
addDatabaseSourceArchiveFolderSpy = jest.spyOn(
726732
databaseManager,
727733
"addDatabaseSourceArchiveFolder",
@@ -746,6 +752,19 @@ describe("local databases", () => {
746752
expect(resolveDatabaseContentsSpy).toBeCalledTimes(1);
747753
});
748754

755+
it("should set the database as the currently selected one", async () => {
756+
const makeSelected = true;
757+
758+
await databaseManager.openDatabase(
759+
{} as ProgressCallback,
760+
{} as CancellationToken,
761+
mockDbItem.databaseUri,
762+
makeSelected,
763+
);
764+
765+
expect(setCurrentDatabaseItemSpy).toBeCalledTimes(1);
766+
});
767+
749768
it("should add database source archive folder", async () => {
750769
await databaseManager.openDatabase(
751770
{} as ProgressCallback,
@@ -762,12 +781,15 @@ describe("local databases", () => {
762781
jest.spyOn(Setting.prototype, "getValue").mockReturnValue(true);
763782

764783
const isTutorialDatabase = true;
784+
const makeSelected = true;
785+
const nameOverride = "CodeQL Tutorial Database";
765786

766787
await databaseManager.openDatabase(
767788
{} as ProgressCallback,
768789
{} as CancellationToken,
769790
mockDbItem.databaseUri,
770-
"CodeQL Tutorial Database",
791+
makeSelected,
792+
nameOverride,
771793
isTutorialDatabase,
772794
);
773795

0 commit comments

Comments
 (0)