From b63fdfaff3d1ffd800b61a54090f6b8af7dca5fc Mon Sep 17 00:00:00 2001 From: Sebastian Malton Date: Thu, 22 Apr 2021 03:05:29 -0400 Subject: [PATCH] Improve documentation of Singleton functions, change to createInstance (#2585) Signed-off-by: Sebastian Malton --- src/common/__tests__/cluster-store.test.ts | 16 ++++---- src/common/__tests__/hotbar-store.test.ts | 4 +- src/common/__tests__/user-store.test.ts | 4 +- src/common/utils/singleton.ts | 41 +++++++++++++++---- .../__tests__/extension-discovery.test.ts | 6 +-- .../__tests__/extension-loader.test.ts | 4 +- src/main/__test__/kube-auth-proxy.test.ts | 2 +- src/main/index.ts | 39 +++++++++--------- .../protocol-handler/__test__/router.test.ts | 6 +-- src/renderer/bootstrap.tsx | 18 ++++---- .../+extensions/__tests__/extensions.test.tsx | 8 ++-- .../__test__/log-resource-selector.test.tsx | 4 +- .../__test__/main-layout-header.test.tsx | 2 +- src/renderer/lens-app.tsx | 2 +- .../utils/__tests__/storageHelper.test.ts | 2 +- 15 files changed, 90 insertions(+), 68 deletions(-) diff --git a/src/common/__tests__/cluster-store.test.ts b/src/common/__tests__/cluster-store.test.ts index fc1b36c548..2dc36b795c 100644 --- a/src/common/__tests__/cluster-store.test.ts +++ b/src/common/__tests__/cluster-store.test.ts @@ -59,7 +59,7 @@ describe("empty config", () => { mockFs(mockOpts); - await ClusterStore.getInstanceOrCreate().load(); + await ClusterStore.createInstance().load(); }); afterEach(() => { @@ -177,7 +177,7 @@ describe("config with existing clusters", () => { mockFs(mockOpts); - return ClusterStore.getInstanceOrCreate().load(); + return ClusterStore.createInstance().load(); }); afterEach(() => { @@ -274,7 +274,7 @@ users: mockFs(mockOpts); - return ClusterStore.getInstanceOrCreate().load(); + return ClusterStore.createInstance().load(); }); afterEach(() => { @@ -316,7 +316,7 @@ describe("pre 2.0 config with an existing cluster", () => { mockFs(mockOpts); - return ClusterStore.getInstanceOrCreate().load(); + return ClusterStore.createInstance().load(); }); afterEach(() => { @@ -386,7 +386,7 @@ describe("pre 2.6.0 config with a cluster that has arrays in auth config", () => mockFs(mockOpts); - return ClusterStore.getInstanceOrCreate().load(); + return ClusterStore.createInstance().load(); }); afterEach(() => { @@ -430,7 +430,7 @@ describe("pre 2.6.0 config with a cluster icon", () => { mockFs(mockOpts); - return ClusterStore.getInstanceOrCreate().load(); + return ClusterStore.createInstance().load(); }); afterEach(() => { @@ -469,7 +469,7 @@ describe("for a pre 2.7.0-beta.0 config without a workspace", () => { mockFs(mockOpts); - return ClusterStore.getInstanceOrCreate().load(); + return ClusterStore.createInstance().load(); }); afterEach(() => { @@ -505,7 +505,7 @@ describe("pre 3.6.0-beta.1 config with an existing cluster", () => { mockFs(mockOpts); - return ClusterStore.getInstanceOrCreate().load(); + return ClusterStore.createInstance().load(); }); afterEach(() => { diff --git a/src/common/__tests__/hotbar-store.test.ts b/src/common/__tests__/hotbar-store.test.ts index 8b245b2a0c..b656f08043 100644 --- a/src/common/__tests__/hotbar-store.test.ts +++ b/src/common/__tests__/hotbar-store.test.ts @@ -5,7 +5,7 @@ import { HotbarStore } from "../hotbar-store"; describe("HotbarStore", () => { beforeEach(() => { ClusterStore.resetInstance(); - ClusterStore.getInstanceOrCreate(); + ClusterStore.createInstance(); HotbarStore.resetInstance(); mockFs({ tmp: { "lens-hotbar-store.json": "{}" } }); @@ -17,7 +17,7 @@ describe("HotbarStore", () => { describe("load", () => { it("loads one hotbar by default", () => { - HotbarStore.getInstanceOrCreate().load(); + HotbarStore.createInstance().load(); expect(HotbarStore.getInstance().hotbars.length).toEqual(1); }); }); diff --git a/src/common/__tests__/user-store.test.ts b/src/common/__tests__/user-store.test.ts index 0bb1b90de4..e4622ffa64 100644 --- a/src/common/__tests__/user-store.test.ts +++ b/src/common/__tests__/user-store.test.ts @@ -28,7 +28,7 @@ describe("user store tests", () => { UserStore.resetInstance(); mockFs({ tmp: { "config.json": "{}", "kube_config": "{}" } }); - (UserStore.getInstanceOrCreate() as any).refreshNewContexts = jest.fn(() => Promise.resolve()); + (UserStore.createInstance() as any).refreshNewContexts = jest.fn(() => Promise.resolve()); return UserStore.getInstance().load(); }); @@ -102,7 +102,7 @@ describe("user store tests", () => { } }); - return UserStore.getInstanceOrCreate().load(); + return UserStore.createInstance().load(); }); afterEach(() => { diff --git a/src/common/utils/singleton.ts b/src/common/utils/singleton.ts index 2f19fd638d..04c0b063a5 100644 --- a/src/common/utils/singleton.ts +++ b/src/common/utils/singleton.ts @@ -1,10 +1,3 @@ -/** - * Narrowing class instances to the one. - * Use "private" or "protected" modifier for constructor (when overriding) to disallow "new" usage. - * - * @example - * const usersStore: UsersStore = UsersStore.getInstance(); - */ type StaticThis = { new(...args: R): T }; export class Singleton { @@ -13,12 +6,28 @@ export class Singleton { constructor() { if (Singleton.creating.length === 0) { - throw new TypeError("A singleton class must be created by getInstanceOrCreate()"); + throw new TypeError("A singleton class must be created by createInstance()"); } } - static getInstanceOrCreate(this: StaticThis, ...args: R): T { + /** + * Creates the single instance of the child class if one was not already created. + * + * Multiple calls will return the same instance. + * Essentially throwing away the arguments to the subsequent calls. + * + * Note: this is a racy function, if two (or more) calls are racing to call this function + * only the first's arguments will be used. + * @param this Implicit argument that is the child class type + * @param args The constructor arguments for the child class + * @returns An instance of the child class + */ + static createInstance(this: StaticThis, ...args: R): T { if (!Singleton.instances.has(this)) { + if (Singleton.creating.length > 0) { + throw new TypeError("Cannot create a second singleton while creating a first"); + } + Singleton.creating = this.name; Singleton.instances.set(this, new this(...args)); Singleton.creating = ""; @@ -27,6 +36,13 @@ export class Singleton { return Singleton.instances.get(this) as T; } + /** + * Get the instance of the child class that was previously created. + * @param this Implicit argument that is the child class type + * @param strict If false will return `undefined` instead of throwing when an instance doesn't exist. + * Default: `true` + * @returns An instance of the child class + */ static getInstance(this: StaticThis, strict = true): T | undefined { if (!Singleton.instances.has(this) && strict) { throw new TypeError(`instance of ${this.name} is not created`); @@ -35,6 +51,13 @@ export class Singleton { return Singleton.instances.get(this) as (T | undefined); } + /** + * Delete the instance of the child class. + * + * Note: this doesn't prevent callers of `getInstance` from storing the result in a global. + * + * There is *no* way in JS or TS to prevent globals like that. + */ static resetInstance() { Singleton.instances.delete(this); } diff --git a/src/extensions/__tests__/extension-discovery.test.ts b/src/extensions/__tests__/extension-discovery.test.ts index d185302b65..c4775f29fe 100644 --- a/src/extensions/__tests__/extension-discovery.test.ts +++ b/src/extensions/__tests__/extension-discovery.test.ts @@ -21,7 +21,7 @@ describe("ExtensionDiscovery", () => { beforeEach(() => { ExtensionDiscovery.resetInstance(); ExtensionsStore.resetInstance(); - ExtensionsStore.getInstanceOrCreate(); + ExtensionsStore.createInstance(); }); it("emits add for added extension", async done => { @@ -43,7 +43,7 @@ describe("ExtensionDiscovery", () => { mockedWatch.mockImplementationOnce(() => (mockWatchInstance) as any ); - const extensionDiscovery = ExtensionDiscovery.getInstanceOrCreate(); + const extensionDiscovery = ExtensionDiscovery.createInstance(); // Need to force isLoaded to be true so that the file watching is started extensionDiscovery.isLoaded = true; @@ -83,7 +83,7 @@ describe("ExtensionDiscovery", () => { mockedWatch.mockImplementationOnce(() => (mockWatchInstance) as any ); - const extensionDiscovery = ExtensionDiscovery.getInstanceOrCreate(); + const extensionDiscovery = ExtensionDiscovery.createInstance(); // Need to force isLoaded to be true so that the file watching is started extensionDiscovery.isLoaded = true; diff --git a/src/extensions/__tests__/extension-loader.test.ts b/src/extensions/__tests__/extension-loader.test.ts index ed7025f218..1115a3d5bc 100644 --- a/src/extensions/__tests__/extension-loader.test.ts +++ b/src/extensions/__tests__/extension-loader.test.ts @@ -110,7 +110,7 @@ describe("ExtensionLoader", () => { }); it.only("renderer updates extension after ipc broadcast", async (done) => { - const extensionLoader = ExtensionLoader.getInstanceOrCreate(); + const extensionLoader = ExtensionLoader.createInstance(); expect(extensionLoader.userExtensions).toMatchInlineSnapshot(`Map {}`); @@ -155,7 +155,7 @@ describe("ExtensionLoader", () => { // Disable sending events in this test (ipcRenderer.on as any).mockImplementation(); - const extensionLoader = ExtensionLoader.getInstanceOrCreate(); + const extensionLoader = ExtensionLoader.createInstance(); await extensionLoader.init(); diff --git a/src/main/__test__/kube-auth-proxy.test.ts b/src/main/__test__/kube-auth-proxy.test.ts index dbed900fd7..612db10a01 100644 --- a/src/main/__test__/kube-auth-proxy.test.ts +++ b/src/main/__test__/kube-auth-proxy.test.ts @@ -50,7 +50,7 @@ describe("kube auth proxy tests", () => { beforeEach(() => { jest.clearAllMocks(); UserStore.resetInstance(); - UserStore.getInstanceOrCreate(); + UserStore.createInstance(); }); it("calling exit multiple times shouldn't throw", async () => { diff --git a/src/main/index.ts b/src/main/index.ts index b1868ad29d..7220f120dd 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -62,7 +62,7 @@ if (app.commandLine.getSwitchValue("proxy-server") !== "") { if (!app.requestSingleInstanceLock()) { app.exit(); } else { - const lprm = LensProtocolRouterMain.getInstanceOrCreate(); + const lprm = LensProtocolRouterMain.createInstance(); for (const arg of process.argv) { if (arg.toLowerCase().startsWith("lens://")) { @@ -73,7 +73,7 @@ if (!app.requestSingleInstanceLock()) { } app.on("second-instance", (event, argv) => { - const lprm = LensProtocolRouterMain.getInstanceOrCreate(); + const lprm = LensProtocolRouterMain.createInstance(); for (const arg of argv) { if (arg.toLowerCase().startsWith("lens://")) { @@ -98,11 +98,11 @@ app.on("ready", async () => { registerFileProtocol("static", __static); - const userStore = UserStore.getInstanceOrCreate(); - const clusterStore = ClusterStore.getInstanceOrCreate(); - const hotbarStore = HotbarStore.getInstanceOrCreate(); - const extensionsStore = ExtensionsStore.getInstanceOrCreate(); - const filesystemStore = FilesystemProvisionerStore.getInstanceOrCreate(); + const userStore = UserStore.createInstance(); + const clusterStore = ClusterStore.createInstance(); + const hotbarStore = HotbarStore.createInstance(); + const extensionsStore = ExtensionsStore.createInstance(); + const filesystemStore = FilesystemProvisionerStore.createInstance(); logger.info("💾 Loading stores"); // preload @@ -114,36 +114,35 @@ app.on("ready", async () => { filesystemStore.load(), ]); - // find free port - let proxyPort; - try { logger.info("🔑 Getting free port for LensProxy server"); - proxyPort = await getFreePort(); + const proxyPort = await getFreePort(); + + // create cluster manager + ClusterManager.createInstance(proxyPort); } catch (error) { logger.error(error); dialog.showErrorBox("Lens Error", "Could not find a free port for the cluster proxy"); app.exit(); } - // create cluster manager - ClusterManager.getInstanceOrCreate(proxyPort); + const clusterManager = ClusterManager.getInstance(); // run proxy try { logger.info("🔌 Starting LensProxy"); // eslint-disable-next-line unused-imports/no-unused-vars-ts - LensProxy.getInstanceOrCreate(proxyPort).listen(); + LensProxy.createInstance(clusterManager.port).listen(); } catch (error) { - logger.error(`Could not start proxy (127.0.0:${proxyPort}): ${error?.message}`); - dialog.showErrorBox("Lens Error", `Could not start proxy (127.0.0:${proxyPort}): ${error?.message || "unknown error"}`); + logger.error(`Could not start proxy (127.0.0:${clusterManager.port}): ${error?.message}`); + dialog.showErrorBox("Lens Error", `Could not start proxy (127.0.0:${clusterManager.port}): ${error?.message || "unknown error"}`); app.exit(); } // test proxy connection try { logger.info("🔎 Testing LensProxy connection ..."); - const versionFromProxy = await getAppVersionFromProxyServer(proxyPort); + const versionFromProxy = await getAppVersionFromProxyServer(clusterManager.port); if (getAppVersion() !== versionFromProxy) { logger.error(`Proxy server responded with invalid response`); @@ -153,9 +152,9 @@ app.on("ready", async () => { logger.error("Checking proxy server connection failed", error); } - const extensionDiscovery = ExtensionDiscovery.getInstanceOrCreate(); + const extensionDiscovery = ExtensionDiscovery.createInstance(); - ExtensionLoader.getInstanceOrCreate().init(); + ExtensionLoader.createInstance().init(); extensionDiscovery.init(); // Start the app without showing the main window when auto starting on login @@ -163,7 +162,7 @@ app.on("ready", async () => { const startHidden = process.argv.includes("--hidden") || (isMac && app.getLoginItemSettings().wasOpenedAsHidden); logger.info("🖥️ Starting WindowManager"); - const windowManager = WindowManager.getInstanceOrCreate(proxyPort); + const windowManager = WindowManager.createInstance(clusterManager.port); installDeveloperTools(); diff --git a/src/main/protocol-handler/__test__/router.test.ts b/src/main/protocol-handler/__test__/router.test.ts index 88cc4fec44..408fde02aa 100644 --- a/src/main/protocol-handler/__test__/router.test.ts +++ b/src/main/protocol-handler/__test__/router.test.ts @@ -17,10 +17,10 @@ function throwIfDefined(val: any): void { describe("protocol router tests", () => { beforeEach(() => { - ExtensionsStore.getInstanceOrCreate(); - ExtensionLoader.getInstanceOrCreate(); + ExtensionsStore.createInstance(); + ExtensionLoader.createInstance(); - const lpr = LensProtocolRouterMain.getInstanceOrCreate(); + const lpr = LensProtocolRouterMain.createInstance(); lpr.extensionsLoaded = true; lpr.rendererLoaded = true; diff --git a/src/renderer/bootstrap.tsx b/src/renderer/bootstrap.tsx index a6fbfdee40..c6e834ed68 100644 --- a/src/renderer/bootstrap.tsx +++ b/src/renderer/bootstrap.tsx @@ -51,16 +51,16 @@ export async function bootstrap(App: AppComponent) { await attachChromeDebugger(); rootElem.classList.toggle("is-mac", isMac); - ExtensionLoader.getInstanceOrCreate().init(); - ExtensionDiscovery.getInstanceOrCreate().init(); + ExtensionLoader.createInstance().init(); + ExtensionDiscovery.createInstance().init(); - const userStore = UserStore.getInstanceOrCreate(); - const clusterStore = ClusterStore.getInstanceOrCreate(); - const extensionsStore = ExtensionsStore.getInstanceOrCreate(); - const filesystemStore = FilesystemProvisionerStore.getInstanceOrCreate(); - const themeStore = ThemeStore.getInstanceOrCreate(); - const hotbarStore = HotbarStore.getInstanceOrCreate(); - const helmRepoManager = HelmRepoManager.getInstanceOrCreate(); + const userStore = UserStore.createInstance(); + const clusterStore = ClusterStore.createInstance(); + const extensionsStore = ExtensionsStore.createInstance(); + const filesystemStore = FilesystemProvisionerStore.createInstance(); + const themeStore = ThemeStore.createInstance(); + const hotbarStore = HotbarStore.createInstance(); + const helmRepoManager = HelmRepoManager.createInstance(); // preload common stores await Promise.all([ diff --git a/src/renderer/components/+extensions/__tests__/extensions.test.tsx b/src/renderer/components/+extensions/__tests__/extensions.test.tsx index d51b1223f0..6e56c112ca 100644 --- a/src/renderer/components/+extensions/__tests__/extensions.test.tsx +++ b/src/renderer/components/+extensions/__tests__/extensions.test.tsx @@ -42,16 +42,16 @@ describe("Extensions", () => { UserStore.resetInstance(); ThemeStore.resetInstance(); - await UserStore.getInstanceOrCreate().load(); - await ThemeStore.getInstanceOrCreate().init(); + await UserStore.createInstance().load(); + await ThemeStore.createInstance().init(); ExtensionLoader.resetInstance(); ExtensionDiscovery.resetInstance(); Extensions.installStates.clear(); - ExtensionDiscovery.getInstanceOrCreate().uninstallExtension = jest.fn(() => Promise.resolve()); + ExtensionDiscovery.createInstance().uninstallExtension = jest.fn(() => Promise.resolve()); - ExtensionLoader.getInstanceOrCreate().addExtension({ + ExtensionLoader.createInstance().addExtension({ id: "extensionId", manifest: { name: "test", diff --git a/src/renderer/components/dock/__test__/log-resource-selector.test.tsx b/src/renderer/components/dock/__test__/log-resource-selector.test.tsx index 90f4b79972..ac57f50ef7 100644 --- a/src/renderer/components/dock/__test__/log-resource-selector.test.tsx +++ b/src/renderer/components/dock/__test__/log-resource-selector.test.tsx @@ -44,8 +44,8 @@ const getFewPodsTabData = (): LogTabData => { describe("", () => { beforeEach(() => { - UserStore.getInstanceOrCreate(); - ThemeStore.getInstanceOrCreate(); + UserStore.createInstance(); + ThemeStore.createInstance(); }); afterEach(() => { diff --git a/src/renderer/components/layout/__test__/main-layout-header.test.tsx b/src/renderer/components/layout/__test__/main-layout-header.test.tsx index d639db92ab..11985e9b42 100644 --- a/src/renderer/components/layout/__test__/main-layout-header.test.tsx +++ b/src/renderer/components/layout/__test__/main-layout-header.test.tsx @@ -16,7 +16,7 @@ const cluster: Cluster = new Cluster({ describe("", () => { beforeEach(() => { - ClusterStore.getInstanceOrCreate(); + ClusterStore.createInstance(); }); afterEach(() => { diff --git a/src/renderer/lens-app.tsx b/src/renderer/lens-app.tsx index 85f31e6eab..adbc32c9e8 100644 --- a/src/renderer/lens-app.tsx +++ b/src/renderer/lens-app.tsx @@ -25,7 +25,7 @@ export class LensApp extends React.Component { static async init() { catalogEntityRegistry.init(); ExtensionLoader.getInstance().loadOnClusterManagerRenderer(); - LensProtocolRouterRenderer.getInstanceOrCreate().init(); + LensProtocolRouterRenderer.createInstance().init(); bindProtocolAddRouteHandlers(); window.addEventListener("offline", () => broadcastMessage("network:offline")); diff --git a/src/renderer/utils/__tests__/storageHelper.test.ts b/src/renderer/utils/__tests__/storageHelper.test.ts index 352040e0f4..b788281cb3 100644 --- a/src/renderer/utils/__tests__/storageHelper.test.ts +++ b/src/renderer/utils/__tests__/storageHelper.test.ts @@ -5,7 +5,7 @@ import { ClusterStore } from "../../../common/cluster-store"; describe("renderer/utils/StorageHelper", () => { beforeEach(() => { - ClusterStore.getInstanceOrCreate(); + ClusterStore.createInstance(); }); afterEach(() => {