From 76db520ce7aa2077c5585f4ab50030af5c68cd7b Mon Sep 17 00:00:00 2001 From: Koen Vlaswinkel Date: Wed, 11 Oct 2023 11:51:05 +0200 Subject: [PATCH 1/2] Only allow a single model editor per database This will add checks in the appropriate places to ensure that only a single model editor is opened per database. --- .../src/model-editor/model-editor-module.ts | 20 ++++++++++++++++ .../src/model-editor/model-editor-view.ts | 24 +++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/extensions/ql-vscode/src/model-editor/model-editor-module.ts b/extensions/ql-vscode/src/model-editor/model-editor-module.ts index 53101f4ba8c..3140daf71bc 100644 --- a/extensions/ql-vscode/src/model-editor/model-editor-module.ts +++ b/extensions/ql-vscode/src/model-editor/model-editor-module.ts @@ -128,6 +128,15 @@ export class ModelEditorModule extends DisposableObject { return; } + const existingViews = this.editorViewTracker.getViews( + db.databaseUri.toString(), + ); + if (existingViews.length > 0) { + await Promise.all(existingViews.map((view) => view.focusView())); + + return; + } + return withProgress( async (progress) => { const maxStep = 4; @@ -192,6 +201,17 @@ export class ModelEditorModule extends DisposableObject { maxStep, }); + // Check again just before opening the editor to ensure no model editor has been opened between + // our first check and now. + const existingViews = this.editorViewTracker.getViews( + db.databaseUri.toString(), + ); + if (existingViews.length > 0) { + await Promise.all(existingViews.map((view) => view.focusView())); + + return; + } + const view = new ModelEditorView( this.app, this.modelingStore, diff --git a/extensions/ql-vscode/src/model-editor/model-editor-view.ts b/extensions/ql-vscode/src/model-editor/model-editor-view.ts index 4e0fc8f626e..f2cad6028a6 100644 --- a/extensions/ql-vscode/src/model-editor/model-editor-view.ts +++ b/extensions/ql-vscode/src/model-editor/model-editor-view.ts @@ -352,6 +352,10 @@ export class ModelEditorView extends AbstractWebview< return this.databaseItem.databaseUri.toString(); } + public async focusView(): Promise { + this.panel?.reveal(); + } + public async revealMethod(method: Method): Promise { this.panel?.reveal(); @@ -520,6 +524,15 @@ export class ModelEditorView extends AbstractWebview< return; } + let existingViews = this.viewTracker.getViews( + addedDatabase.databaseUri.toString(), + ); + if (existingViews.length > 0) { + await Promise.all(existingViews.map((view) => view.focusView())); + + return; + } + const modelFile = await pickExtensionPack( this.cliServer, addedDatabase, @@ -532,6 +545,17 @@ export class ModelEditorView extends AbstractWebview< return; } + // Check again just before opening the editor to ensure no model editor has been opened between + // our first check and now. + existingViews = this.viewTracker.getViews( + addedDatabase.databaseUri.toString(), + ); + if (existingViews.length > 0) { + await Promise.all(existingViews.map((view) => view.focusView())); + + return; + } + const view = new ModelEditorView( this.app, this.modelingStore, From 039b28235d6996fc0c7e7dc847e20a1e0d7b140b Mon Sep 17 00:00:00 2001 From: Koen Vlaswinkel Date: Wed, 11 Oct 2023 11:54:47 +0200 Subject: [PATCH 2/2] Only allow 1 view in the view tracker This will change the model editor view tracker to only store 1 view per database item instead of an array of views per database item. --- .../method-modeling-view-provider.ts | 8 ++----- .../src/model-editor/model-editor-module.ts | 12 +++++----- .../model-editor/model-editor-view-tracker.ts | 22 ++++++------------- .../src/model-editor/model-editor-view.ts | 12 +++++----- .../modelEditorViewTrackerMock.ts | 6 ++--- 5 files changed, 24 insertions(+), 36 deletions(-) diff --git a/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts b/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts index 571db56d769..040ff1555ec 100644 --- a/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts +++ b/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts @@ -149,14 +149,10 @@ export class MethodModelingViewProvider extends AbstractWebviewViewProvider< return; } - const views = this.editorViewTracker.getViews( + const view = this.editorViewTracker.getView( this.databaseItem.databaseUri.toString(), ); - if (views.length === 0) { - return; - } - - await Promise.all(views.map((view) => view.revealMethod(method))); + await view?.revealMethod(method); } private registerToModelingStoreEvents(): void { diff --git a/extensions/ql-vscode/src/model-editor/model-editor-module.ts b/extensions/ql-vscode/src/model-editor/model-editor-module.ts index 3140daf71bc..d2853a07435 100644 --- a/extensions/ql-vscode/src/model-editor/model-editor-module.ts +++ b/extensions/ql-vscode/src/model-editor/model-editor-module.ts @@ -128,11 +128,11 @@ export class ModelEditorModule extends DisposableObject { return; } - const existingViews = this.editorViewTracker.getViews( + const existingView = this.editorViewTracker.getView( db.databaseUri.toString(), ); - if (existingViews.length > 0) { - await Promise.all(existingViews.map((view) => view.focusView())); + if (existingView) { + await existingView.focusView(); return; } @@ -203,11 +203,11 @@ export class ModelEditorModule extends DisposableObject { // Check again just before opening the editor to ensure no model editor has been opened between // our first check and now. - const existingViews = this.editorViewTracker.getViews( + const existingView = this.editorViewTracker.getView( db.databaseUri.toString(), ); - if (existingViews.length > 0) { - await Promise.all(existingViews.map((view) => view.focusView())); + if (existingView) { + await existingView.focusView(); return; } diff --git a/extensions/ql-vscode/src/model-editor/model-editor-view-tracker.ts b/extensions/ql-vscode/src/model-editor/model-editor-view-tracker.ts index b04739f4704..b4cf12978de 100644 --- a/extensions/ql-vscode/src/model-editor/model-editor-view-tracker.ts +++ b/extensions/ql-vscode/src/model-editor/model-editor-view-tracker.ts @@ -9,33 +9,25 @@ interface ModelEditorViewInterface { export class ModelEditorViewTracker< T extends ModelEditorViewInterface = ModelEditorViewInterface, > { - private readonly views = new Map(); + private readonly views = new Map(); constructor() {} public registerView(view: T): void { const databaseUri = view.databaseUri; - if (!this.views.has(databaseUri)) { - this.views.set(databaseUri, []); + if (this.views.has(databaseUri)) { + throw new Error(`View for database ${databaseUri} already registered`); } - this.views.get(databaseUri)?.push(view); + this.views.set(databaseUri, view); } public unregisterView(view: T): void { - const views = this.views.get(view.databaseUri); - if (!views) { - return; - } - - const index = views.indexOf(view); - if (index !== -1) { - views.splice(index, 1); - } + this.views.delete(view.databaseUri); } - public getViews(databaseUri: string): T[] { - return this.views.get(databaseUri) ?? []; + public getView(databaseUri: string): T | undefined { + return this.views.get(databaseUri); } } diff --git a/extensions/ql-vscode/src/model-editor/model-editor-view.ts b/extensions/ql-vscode/src/model-editor/model-editor-view.ts index f2cad6028a6..62b53365f50 100644 --- a/extensions/ql-vscode/src/model-editor/model-editor-view.ts +++ b/extensions/ql-vscode/src/model-editor/model-editor-view.ts @@ -524,11 +524,11 @@ export class ModelEditorView extends AbstractWebview< return; } - let existingViews = this.viewTracker.getViews( + let existingView = this.viewTracker.getView( addedDatabase.databaseUri.toString(), ); - if (existingViews.length > 0) { - await Promise.all(existingViews.map((view) => view.focusView())); + if (existingView) { + await existingView.focusView(); return; } @@ -547,11 +547,11 @@ export class ModelEditorView extends AbstractWebview< // Check again just before opening the editor to ensure no model editor has been opened between // our first check and now. - existingViews = this.viewTracker.getViews( + existingView = this.viewTracker.getView( addedDatabase.databaseUri.toString(), ); - if (existingViews.length > 0) { - await Promise.all(existingViews.map((view) => view.focusView())); + if (existingView) { + await existingView.focusView(); return; } diff --git a/extensions/ql-vscode/test/__mocks__/model-editor/modelEditorViewTrackerMock.ts b/extensions/ql-vscode/test/__mocks__/model-editor/modelEditorViewTrackerMock.ts index aed1abf1b03..9a4655c6df9 100644 --- a/extensions/ql-vscode/test/__mocks__/model-editor/modelEditorViewTrackerMock.ts +++ b/extensions/ql-vscode/test/__mocks__/model-editor/modelEditorViewTrackerMock.ts @@ -5,15 +5,15 @@ import { ModelEditorView } from "../../../src/model-editor/model-editor-view"; export function createMockModelEditorViewTracker({ registerView = jest.fn(), unregisterView = jest.fn(), - getViews = jest.fn(), + getView = jest.fn(), }: { registerView?: ModelEditorViewTracker["registerView"]; unregisterView?: ModelEditorViewTracker["unregisterView"]; - getViews?: ModelEditorViewTracker["getViews"]; + getView?: ModelEditorViewTracker["getView"]; } = {}): ModelEditorViewTracker { return mockedObject>({ registerView, unregisterView, - getViews, + getView, }); }