diff --git a/extensions/ql-vscode/CHANGELOG.md b/extensions/ql-vscode/CHANGELOG.md index 4ff96f90f..bb3311a05 100644 --- a/extensions/ql-vscode/CHANGELOG.md +++ b/extensions/ql-vscode/CHANGELOG.md @@ -2,6 +2,8 @@ ## [UNRELEASED] +- Fix a bug when re-importing test databases that erroneously showed old source code. [#3616](https://github.com/github/vscode-codeql/pull/3616) + ## 1.13.0 - 1 May 2024 - Add Ruby support to the CodeQL Model Editor. [#3584](https://github.com/github/vscode-codeql/pull/3584) diff --git a/extensions/ql-vscode/src/common/vscode/archive-filesystem-provider.ts b/extensions/ql-vscode/src/common/vscode/archive-filesystem-provider.ts index 3ccdc3668..c82225f42 100644 --- a/extensions/ql-vscode/src/common/vscode/archive-filesystem-provider.ts +++ b/extensions/ql-vscode/src/common/vscode/archive-filesystem-provider.ts @@ -26,6 +26,8 @@ import { // All path operations in this file must be on paths *within* the zip // archive. import { posix } from "path"; +import { DatabaseEventKind } from "../../databases/local-databases/database-events"; +import type { DatabaseManager } from "../../databases/local-databases/database-manager"; const path = posix; @@ -242,15 +244,8 @@ export class ArchiveFileSystemProvider implements FileSystemProvider { root = new Directory(""); - constructor() { - // When a file system archive is removed from the workspace, we should - // also remove it from our cache. - workspace.onDidChangeWorkspaceFolders((event) => { - for (const removed of event.removed) { - const zipPath = removed.uri.fsPath; - this.archives.delete(zipPath); - } - }); + flushCache(zipPath: string) { + this.archives.delete(zipPath); } // metadata @@ -366,15 +361,35 @@ export class ArchiveFileSystemProvider implements FileSystemProvider { */ export const zipArchiveScheme = "codeql-zip-archive"; -export function activate(ctx: ExtensionContext) { +export function activate(ctx: ExtensionContext, dbm?: DatabaseManager) { + const afsp = new ArchiveFileSystemProvider(); + if (dbm) { + ctx.subscriptions.push( + dbm.onDidChangeDatabaseItem(async ({ kind, item: db }) => { + if (kind === DatabaseEventKind.Remove) { + if (db?.sourceArchive) { + afsp.flushCache(db.sourceArchive.fsPath); + } + } + }), + ); + } + ctx.subscriptions.push( - workspace.registerFileSystemProvider( - zipArchiveScheme, - new ArchiveFileSystemProvider(), - { - isCaseSensitive: true, - isReadonly: true, - }, - ), + // When a file system archive is removed from the workspace, we should + // also remove it from our cache. + workspace.onDidChangeWorkspaceFolders((event) => { + for (const removed of event.removed) { + const zipPath = removed.uri.fsPath; + afsp.flushCache(zipPath); + } + }), + ); + + ctx.subscriptions.push( + workspace.registerFileSystemProvider(zipArchiveScheme, afsp, { + isCaseSensitive: true, + isReadonly: true, + }), ); } diff --git a/extensions/ql-vscode/src/databases/local-databases-ui.ts b/extensions/ql-vscode/src/databases/local-databases-ui.ts index 91e0994cb..8a1298395 100644 --- a/extensions/ql-vscode/src/databases/local-databases-ui.ts +++ b/extensions/ql-vscode/src/databases/local-databases-ui.ts @@ -109,9 +109,8 @@ class DatabaseTreeDataProvider // Note that events from the database manager are instances of DatabaseChangedEvent // and events fired by the UI are instances of DatabaseItem - // When event.item is undefined, then the entire tree is refreshed. - // When event.item is a db item, then only that item is refreshed. - this._onDidChangeTreeData.fire(event.item); + // When a full refresh has occurred, then all items are refreshed by passing undefined. + this._onDidChangeTreeData.fire(event.fullRefresh ? undefined : event.item); } private handleDidChangeCurrentDatabaseItem( diff --git a/extensions/ql-vscode/src/databases/local-databases/database-events.ts b/extensions/ql-vscode/src/databases/local-databases/database-events.ts index 41c2179a1..d41a9f319 100644 --- a/extensions/ql-vscode/src/databases/local-databases/database-events.ts +++ b/extensions/ql-vscode/src/databases/local-databases/database-events.ts @@ -16,4 +16,8 @@ export enum DatabaseEventKind { export interface DatabaseChangedEvent { kind: DatabaseEventKind; item: DatabaseItem | undefined; + // If true, event handlers should consider the database manager + // to have been fully refreshed. Any state managed by the + // event handler should be fully refreshed as well. + fullRefresh: boolean; } 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 b766fae7d..7b4eb482f 100644 --- a/extensions/ql-vscode/src/databases/local-databases/database-manager.ts +++ b/extensions/ql-vscode/src/databases/local-databases/database-manager.ts @@ -613,6 +613,7 @@ export class DatabaseManager extends DisposableObject { this._onDidChangeCurrentDatabaseItem.fire({ item, kind: DatabaseEventKind.Change, + fullRefresh: false, }); } } @@ -662,8 +663,9 @@ export class DatabaseManager extends DisposableObject { } // note that we use undefined as the item in order to reset the entire tree this._onDidChangeDatabaseItem.fire({ - item: undefined, + item, kind: DatabaseEventKind.Add, + fullRefresh: true, }); } @@ -671,9 +673,9 @@ export class DatabaseManager extends DisposableObject { item.name = newName; await this.updatePersistedDatabaseList(); this._onDidChangeDatabaseItem.fire({ - // pass undefined so that the entire tree is rebuilt in order to re-sort - item: undefined, + item, kind: DatabaseEventKind.Rename, + fullRefresh: true, }); } @@ -720,10 +722,10 @@ export class DatabaseManager extends DisposableObject { ); } - // note that we use undefined as the item in order to reset the entire tree this._onDidChangeDatabaseItem.fire({ - item: undefined, + item, kind: DatabaseEventKind.Remove, + fullRefresh: true, }); } @@ -776,6 +778,7 @@ export class DatabaseManager extends DisposableObject { this._onDidChangeDatabaseItem.fire({ kind: DatabaseEventKind.Refresh, item: databaseItem, + fullRefresh: false, }); } } diff --git a/extensions/ql-vscode/src/extension.ts b/extensions/ql-vscode/src/extension.ts index 59a0c4977..2b67b374d 100644 --- a/extensions/ql-vscode/src/extension.ts +++ b/extensions/ql-vscode/src/extension.ts @@ -947,7 +947,7 @@ async function activateWithInstalledDistribution( ctx.subscriptions.push(compareView); void extLogger.log("Initializing source archive filesystem provider."); - archiveFilesystemProvider_activate(ctx); + archiveFilesystemProvider_activate(ctx, dbm); const qhelpTmpDir = dirSync({ prefix: "qhelp_", diff --git a/extensions/ql-vscode/test/vscode-tests/minimal-workspace/local-queries/local-databases.test.ts b/extensions/ql-vscode/test/vscode-tests/minimal-workspace/local-queries/local-databases.test.ts index 66fcd8093..b72be5488 100644 --- a/extensions/ql-vscode/test/vscode-tests/minimal-workspace/local-queries/local-databases.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/minimal-workspace/local-queries/local-databases.test.ts @@ -140,7 +140,8 @@ describe("local databases", () => { }, ]); expect(onDidChangeDatabaseItem).toHaveBeenCalledWith({ - item: undefined, + fullRefresh: true, + item: mockDbItem, kind: DatabaseEventKind.Add, }); @@ -152,7 +153,8 @@ describe("local databases", () => { expect((databaseManager as any)._databaseItems).toEqual([]); expect(updateSpy).toHaveBeenCalledWith("databaseList", []); expect(onDidChangeDatabaseItem).toHaveBeenCalledWith({ - item: undefined, + fullRefresh: true, + item: mockDbItem, kind: DatabaseEventKind.Remove, }); }); @@ -175,7 +177,8 @@ describe("local databases", () => { ]); expect(onDidChangeDatabaseItem).toHaveBeenCalledWith({ - item: undefined, + fullRefresh: true, + item: mockDbItem, kind: DatabaseEventKind.Rename, }); }); @@ -198,7 +201,8 @@ describe("local databases", () => { ]); const mockEvent = { - item: undefined, + fullRefresh: true, + item: mockDbItem, kind: DatabaseEventKind.Add, }; expect(onDidChangeDatabaseItem).toHaveBeenCalledWith(mockEvent);