From 2ef2cbb2dfc04f2c0d58235fa0501b71a548d21e Mon Sep 17 00:00:00 2001 From: Sebastian Malton Date: Wed, 28 Jul 2021 14:55:02 -0400 Subject: [PATCH] Fix race condition with multiple opens Signed-off-by: Sebastian Malton --- .../catalog-entities/kubernetes-cluster.ts | 52 ++++++++++++------- src/common/catalog/catalog-entity.ts | 3 +- src/common/utils/index.ts | 1 + src/common/utils/map-helpers.ts | 35 +++++++++++++ src/main/initializers/ipc.ts | 41 +++++++++------ 5 files changed, 97 insertions(+), 35 deletions(-) create mode 100644 src/common/utils/map-helpers.ts diff --git a/src/common/catalog-entities/kubernetes-cluster.ts b/src/common/catalog-entities/kubernetes-cluster.ts index fb2bac0e06..7ac3394b3a 100644 --- a/src/common/catalog-entities/kubernetes-cluster.ts +++ b/src/common/catalog-entities/kubernetes-cluster.ts @@ -23,13 +23,17 @@ import { catalogCategoryRegistry } from "../catalog/catalog-category-registry"; import { CatalogEntity, CatalogEntityAddMenuContext, CatalogEntityContextMenuContext, CatalogEntityMetadata, CatalogEntityStatus } from "../catalog"; import { clusterActivateHandler, clusterDeleteHandler, clusterDisconnectHandler, navigateToClusterHandler } from "../cluster-ipc"; import { ClusterStore } from "../cluster-store"; -import { onNewWindowForClusterHandler, requestMain } from "../ipc"; +import { requestMain } from "../ipc"; import { CatalogCategory, CatalogCategorySpec } from "../catalog"; import { addClusterURL } from "../routes"; import { app } from "electron"; -import type { CatalogEntitySpec } from "../catalog/catalog-entity"; +import type { CatalogEntityActionContext, CatalogEntitySpec } from "../catalog/catalog-entity"; import { HotbarStore } from "../hotbar-store"; +export interface KubernetesClusterActionContextArgs extends Record { + newWindow?: boolean; +} + export interface KubernetesClusterPrometheusMetrics { address?: { namespace: string; @@ -90,8 +94,12 @@ export class KubernetesCluster extends CatalogEntity) { + await requestMain( + navigateToClusterHandler, + this.getId(), + context.args?.newWindow ?? false, + ); } onDetailsOpen(): void { @@ -103,20 +111,28 @@ export class KubernetesCluster extends CatalogEntity this.onRun(), - }, - { - title: "Open in new window", - icon: "launch", - onClick: () => requestMain(onNewWindowForClusterHandler, this.getId()), - }, - ); - } + const runContext = () => ({ + navigate: context.navigate, + setCommandPaletteContext: context.setCommandPaletteContext, + }); + + context.menuItems.push( + { + title: "Open", + icon: "open_in_full", + onClick: () => this.onRun(runContext()), + }, + { + title: "Open in new window", + icon: "launch", + onClick: () => this.onRun({ + ...runContext(), + args: { + newWindow: true, + } + }), + }, + ); if (!this.metadata.source || this.metadata.source === "local") { context.menuItems.push( diff --git a/src/common/catalog/catalog-entity.ts b/src/common/catalog/catalog-entity.ts index 1c2d0a2061..3d5e4b182c 100644 --- a/src/common/catalog/catalog-entity.ts +++ b/src/common/catalog/catalog-entity.ts @@ -95,9 +95,10 @@ export interface CatalogEntityStatus { active?: boolean; } -export interface CatalogEntityActionContext { +export interface CatalogEntityActionContext = Record> { navigate: (url: string) => void; setCommandPaletteContext: (context?: CatalogEntity) => void; + args?: T; } export interface CatalogEntityContextMenu { diff --git a/src/common/utils/index.ts b/src/common/utils/index.ts index 220c106444..58e2655e9c 100644 --- a/src/common/utils/index.ts +++ b/src/common/utils/index.ts @@ -51,6 +51,7 @@ export * from "./tar"; export * from "./toggle-set"; export * from "./toJS"; export * from "./type-narrowing"; +export * from "./map-helpers"; import * as iter from "./iter"; diff --git a/src/common/utils/map-helpers.ts b/src/common/utils/map-helpers.ts new file mode 100644 index 0000000000..b4737b2348 --- /dev/null +++ b/src/common/utils/map-helpers.ts @@ -0,0 +1,35 @@ +/** + * Copyright (c) 2021 OpenLens Authors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to + * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of + * the Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER + * IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ + +/** + * Get the current value ascociated with `key` or + * @param map The map to interact with + * @param key The key to optionally initialize + * @param value The value to use if the key needs initializing + * @returns The current value in the map + */ +export function getOrInsert(map: Map, key: K, value: V): V { + if (!map.has(key)) { + map.set(key, value); + } + + return map.get(key); +} diff --git a/src/main/initializers/ipc.ts b/src/main/initializers/ipc.ts index 4b88ba4a34..119ee683af 100644 --- a/src/main/initializers/ipc.ts +++ b/src/main/initializers/ipc.ts @@ -19,20 +19,23 @@ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ +import AwaitLock from "await-lock"; import type { IpcMainInvokeEvent } from "electron"; +import { when } from "mobx"; import { KubernetesCluster } from "../../common/catalog-entities"; import { ClusterFrames } from "../../common/cluster-frames"; import { clusterActivateHandler, clusterSetFrameIdHandler, clusterVisibilityHandler, clusterRefreshHandler, clusterDisconnectHandler, clusterKubectlApplyAllHandler, clusterKubectlDeleteAllHandler, clusterDeleteHandler, navigateToClusterHandler } from "../../common/cluster-ipc"; import { ClusterId, ClusterStore } from "../../common/cluster-store"; import { appEventBus } from "../../common/event-bus"; -import { ipcMainHandle, onNewWindowForClusterHandler } from "../../common/ipc"; +import { ipcMainHandle } from "../../common/ipc"; +import { getOrInsert } from "../../common/utils"; import { catalogEntityRegistry } from "../catalog"; import { ClusterManager } from "../cluster-manager"; import { bundledKubectlPath } from "../kubectl"; import logger from "../logger"; import { promiseExecFile } from "../promise-exec"; import { ResourceApplier } from "../resource-applier"; -import { WindowManager } from "../window-manager"; +import { NavigateFrameInfoSpecifier, WindowManager } from "../window-manager"; export function initIpcMainHandlers() { ipcMainHandle(clusterActivateHandler, (event, clusterId: ClusterId, force = false) => { @@ -54,16 +57,6 @@ export function initIpcMainHandlers() { } }); - ipcMainHandle(navigateToClusterHandler, async (event, clusterId: ClusterId) => { - const cluster = ClusterStore.getInstance().getById(clusterId); - - if (!cluster) { - return void logger.warn("[NAVIGATE-TO-CLUSTER]: unknown cluster", { clusterId }); - } - - await WindowManager.getInstance().navigate(`/cluster/${clusterId}`, { clusterId }, { windowId: event.sender.getProcessId() }); - }); - ipcMainHandle(clusterVisibilityHandler, (event: IpcMainInvokeEvent, clusterId: ClusterId, visible: boolean) => { const entity = catalogEntityRegistry.getById(clusterId); @@ -153,18 +146,34 @@ export function initIpcMainHandlers() { } }); - ipcMainHandle(onNewWindowForClusterHandler, async (event, clusterId: ClusterId) => { - appEventBus.emit({ name: "cluster", action: "open-new-window" }); + const navigateLocks = new Map(); + + ipcMainHandle(navigateToClusterHandler, async (event, clusterId: ClusterId, newWindow?: boolean) => { + appEventBus.emit({ name: "cluster", action: "navigate" }); const cluster = ClusterStore.getInstance().getById(clusterId); if (!cluster) { - return void logger.info("Cannot open clutser in new window, unknown cluster Id", { clusterId }); + return void logger.warn("[NAVIGATE-TO-CLUSTER]: unknown cluster", { clusterId }); } + const lock = getOrInsert(navigateLocks, clusterId, new AwaitLock()); + try { - await WindowManager.getInstance().navigate(`/cluster/${clusterId}`, { clusterId }, { windowId: true }); + await lock.acquireAsync(); + const specifics: NavigateFrameInfoSpecifier[] = [{ clusterId }]; + + if (newWindow) { + specifics.push({ windowId: true }); + } else { + specifics.push({ windowId: event.sender.getProcessId() }); + } + + await WindowManager.getInstance().navigate(`/cluster/${clusterId}`, ...specifics); + await when(() => Boolean(ClusterFrames.getInstance().getFrameInfoByClusterId(clusterId))); } catch (error) { logger.error("Failed to load url for new cluster window", error); + } finally { + lock.release(); } }); }