Remove Singleton from BaseStore to remove global shared state

Signed-off-by: Sebastian Malton <sebastian@malton.name>

src/common/__tests__/base-store.test.ts

@@ -82,8 +82,6 @@ describe("BaseStore", () => {
     mainDi.override(directoryForUserDataInjectable, () => "some-user-data-directory");
     mainDi.permitSideEffects(getConfigurationFileModelInjectable);
 
-    TestStore.resetInstance();
-
     const mockOpts = {
       "some-user-data-directory": {
         "test-store.json": JSON.stringify({}),
@@ -92,13 +90,12 @@ describe("BaseStore", () => {
 
     mockFs(mockOpts);
 
-    store = TestStore.createInstance();
+    store = new TestStore();
   });
 
   afterEach(() => {
     mockFs.restore();
     store.disableSync();
-    TestStore.resetInstance();
   });
 
   describe("persistence", () => {

src/common/base-store.ts

@@ -10,7 +10,7 @@ import { ipcMain, ipcRenderer } from "electron";
 import type { IEqualsComparer } from "mobx";
 import { makeObservable, reaction, runInAction } from "mobx";
 import type { Disposer } from "./utils";
-import { Singleton, toJS } from "./utils";
+import { toJS } from "./utils";
 import logger from "../main/logger";
 import { broadcastMessage, ipcMainOn, ipcRendererOn } from "./ipc";
 import isEqual from "lodash/isEqual";
@@ -31,14 +31,13 @@ export interface BaseStoreParams<T> extends ConfOptions<T> {
 /**
  * Note: T should only contain base JSON serializable types.
  */
-export abstract class BaseStore<T extends object> extends Singleton {
+export abstract class BaseStore<T extends object> {
   protected storeConfig?: Config<T>;
   protected syncDisposers: Disposer[] = [];
 
   readonly displayName: string = this.constructor.name;
 
   protected constructor(protected params: BaseStoreParams<T>) {
-    super();
     makeObservable(this);
 
     if (ipcRenderer) {

src/common/cluster-store/cluster-store.injectable.ts

@@ -11,15 +11,11 @@ import emitAppEventInjectable from "../app-event-bus/emit-event.injectable";
 const clusterStoreInjectable = getInjectable({
   id: "cluster-store",
 
-  instantiate: (di) => {
-    ClusterStore.resetInstance();
-
-    return ClusterStore.createInstance({
-      createCluster: di.inject(createClusterInjectionToken),
-      readClusterConfigSync: di.inject(readClusterConfigSyncInjectable),
-      emitAppEvent: di.inject(emitAppEventInjectable),
-    });
-  },
+  instantiate: (di) => new ClusterStore({
+    createCluster: di.inject(createClusterInjectionToken),
+    readClusterConfigSync: di.inject(readClusterConfigSyncInjectable),
+    emitAppEvent: di.inject(emitAppEventInjectable),
+  }),
 
   causesSideEffects: true,
 });

src/common/hotbars/store.injectable.ts

@@ -10,14 +10,10 @@ import loggerInjectable from "../logger.injectable";
 const hotbarStoreInjectable = getInjectable({
   id: "hotbar-store",
 
-  instantiate: (di) => {
-    HotbarStore.resetInstance();
-
-    return HotbarStore.createInstance({
-      catalogCatalogEntity: di.inject(catalogCatalogEntityInjectable),
-      logger: di.inject(loggerInjectable),
-    });
-  },
+  instantiate: (di) => new HotbarStore({
+    catalogCatalogEntity: di.inject(catalogCatalogEntityInjectable),
+    logger: di.inject(loggerInjectable),
+  }),
 
   causesSideEffects: true,
 });

src/common/user-store/user-store.injectable.ts

@@ -10,14 +10,10 @@ import emitAppEventInjectable from "../app-event-bus/emit-event.injectable";
 const userStoreInjectable = getInjectable({
   id: "user-store",
 
-  instantiate: (di) => {
-    UserStore.resetInstance();
-
-    return UserStore.createInstance({
-      selectedUpdateChannel: di.inject(selectedUpdateChannelInjectable),
-      emitAppEvent: di.inject(emitAppEventInjectable),
-    });
-  },
+  instantiate: (di) => new UserStore({
+    selectedUpdateChannel: di.inject(selectedUpdateChannelInjectable),
+    emitAppEvent: di.inject(emitAppEventInjectable),
+  }),
 
   causesSideEffects: true,
 });

src/common/utils/collection-functions.ts

@@ -52,7 +52,10 @@ export function getOrInsertSet<K, SK>(map: Map<K, Set<SK>>, key: K): Set<SK> {
  * Like `getOrInsert` but with delayed creation of the item. Which is useful
  * if it is very expensive to create the initial value.
  */
-export function getOrInsertWith<K, V>(map: Map<K, V>, key: K, builder: () => V): V {
+export function getOrInsertWith<K, V>(map: Map<K, V>, key: K, builder: () => V): V;
+export function getOrInsertWith<K extends object, V>(map: Map<K, V> | WeakMap<K, V>, key: K, builder: () => V): V;
+
+export function getOrInsertWith<K extends object, V>(map: Map<K, V> | WeakMap<K, V>, key: K, builder: () => V): V {
   if (!map.has(key)) {
     map.set(key, builder());
   }

src/common/utils/random-bytes.injectable.ts

@@ -0,0 +1,16 @@
+/**
+ * Copyright (c) OpenLens Authors. All rights reserved.
+ * Licensed under MIT License. See LICENSE in root directory for more information.
+ */
+import { getInjectable } from "@ogre-tools/injectable";
+import { randomBytes } from "crypto";
+import { promisify } from "util";
+
+export type RandomBytes = (size: number) => Promise<Buffer>;
+
+const randomBytesInjectable = getInjectable({
+  id: "random-bytes",
+  instantiate: (): RandomBytes => promisify(randomBytes),
+});
+
+export default randomBytesInjectable;

src/common/utils/singleton.ts

@@ -3,10 +3,13 @@
  * Licensed under MIT License. See LICENSE in root directory for more information.
  */
 
-interface StaticThis<T, R extends any[]> { new(...args: R): T }
+export interface StaticThis<T, R extends any[]> { new(...args: R): T }
 
+/**
+ * @deprecated This is a form of global shared state
+ */
 export class Singleton {
-  private static instances = new WeakMap<object, Singleton>();
+  private static readonly instances = new WeakMap<object, Singleton>();
   private static creating = "";
 
   constructor() {

src/common/weblink-store.injectable.ts

@@ -7,12 +7,7 @@ import { WeblinkStore } from "./weblink-store";
 
 const weblinkStoreInjectable = getInjectable({
   id: "weblink-store",
-
-  instantiate: () => {
-    WeblinkStore.resetInstance();
-
-    return WeblinkStore.createInstance();
-  },
+  instantiate: () => new WeblinkStore(),
 });
 
 export default weblinkStoreInjectable;

src/extensions/extension-loader/file-system-provisioner-store/file-system-provisioner-store.injectable.ts

@@ -5,19 +5,19 @@
 import { getInjectable } from "@ogre-tools/injectable";
 import { FileSystemProvisionerStore } from "./file-system-provisioner-store";
 import directoryForExtensionDataInjectable from "./directory-for-extension-data.injectable";
+import ensureDirectoryInjectable from "../../../common/fs/ensure-dir.injectable";
+import joinPathsInjectable from "../../../common/path/join-paths.injectable";
+import randomBytesInjectable from "../../../common/utils/random-bytes.injectable";
 
 const fileSystemProvisionerStoreInjectable = getInjectable({
   id: "file-system-provisioner-store",
 
-  instantiate: (di) => {
-    FileSystemProvisionerStore.resetInstance();
-
-    return FileSystemProvisionerStore.createInstance({
-      directoryForExtensionData: di.inject(directoryForExtensionDataInjectable),
-    });
-  },
-
-  causesSideEffects: true,
+  instantiate: (di) => new FileSystemProvisionerStore({
+    directoryForExtensionData: di.inject(directoryForExtensionDataInjectable),
+    ensureDirectory: di.inject(ensureDirectoryInjectable),
+    joinPaths: di.inject(joinPathsInjectable),
+    randomBytes: di.inject(randomBytesInjectable),
+  }),
 });
 
 export default fileSystemProvisionerStoreInjectable;

src/extensions/extension-loader/file-system-provisioner-store/file-system-provisioner-store.ts

@@ -3,28 +3,31 @@
  * Licensed under MIT License. See LICENSE in root directory for more information.
  */
 
-import { randomBytes } from "crypto";
 import { SHA256 } from "crypto-js";
-import fse from "fs-extra";
 import { action, makeObservable, observable } from "mobx";
-import path from "path";
 import { BaseStore } from "../../../common/base-store";
 import type { LensExtensionId } from "../../lens-extension";
-import { getOrInsertWith, toJS } from "../../../common/utils";
+import { getOrInsertWithAsync, toJS } from "../../../common/utils";
+import type { EnsureDirectory } from "../../../common/fs/ensure-dir.injectable";
+import type { JoinPaths } from "../../../common/path/join-paths.injectable";
+import type { RandomBytes } from "../../../common/utils/random-bytes.injectable";
 
 interface FSProvisionModel {
   extensions: Record<string, string>; // extension names to paths
 }
 
 interface Dependencies {
-  directoryForExtensionData: string;
+  readonly directoryForExtensionData: string;
+  ensureDirectory: EnsureDirectory;
+  joinPaths: JoinPaths;
+  randomBytes: RandomBytes;
 }
 
 export class FileSystemProvisionerStore extends BaseStore<FSProvisionModel> {
   readonly displayName = "FilesystemProvisionerStore";
-  registeredExtensions = observable.map<LensExtensionId, string>();
+  readonly registeredExtensions = observable.map<LensExtensionId, string>();
 
-  constructor(private dependencies: Dependencies) {
+  constructor(private readonly dependencies: Dependencies) {
     super({
       configName: "lens-filesystem-provisioner-store",
       accessPropertiesByDotNotation: false, // To make dots safe in cluster context names
@@ -41,14 +44,14 @@ export class FileSystemProvisionerStore extends BaseStore<FSProvisionModel> {
    * @returns path to the folder that the extension can safely write files to.
    */
   async requestDirectory(extensionName: string): Promise<string> {
-    const dirPath = getOrInsertWith(this.registeredExtensions, extensionName, () => {
-      const salt = randomBytes(32).toString("hex");
+    const dirPath = await getOrInsertWithAsync(this.registeredExtensions, extensionName, async () => {
+      const salt = (await this.dependencies.randomBytes(32)).toString("hex");
       const hashedName = SHA256(`${extensionName}/${salt}`).toString();
 
-      return path.resolve(this.dependencies.directoryForExtensionData, hashedName);
+      return this.dependencies.joinPaths(this.dependencies.directoryForExtensionData, hashedName);
     });
 
-    await fse.ensureDir(dirPath);
+    await this.dependencies.ensureDirectory(dirPath);
 
     return dirPath;
   }

src/extensions/extension-store.ts

@@ -7,8 +7,39 @@ import { BaseStore } from "../common/base-store";
 import * as path from "path";
 import type { LensExtension } from "./lens-extension";
 import assert from "assert";
+import type { StaticThis } from "../common/utils";
+import { getOrInsertWith } from "../common/utils";
 
 export abstract class ExtensionStore<T extends object> extends BaseStore<T> {
+  private static readonly instances = new WeakMap<object, ExtensionStore<object>>();
+
+  /**
+   * @deprecated This is a form of global shared state. Just call `new Store(...)`
+   */
+  static createInstance<T extends ExtensionStore<object>, R extends any[]>(this: StaticThis<T, R>, ...args: R): T {
+    return getOrInsertWith(ExtensionStore.instances, this, () =>  new this(...args)) as T;
+  }
+
+  /**
+   * @deprecated This is a form of global shared state. Just call `new Store(...)`
+   */
+  static getInstance<T, R extends any[]>(this: StaticThis<T, R>, strict?: true): T;
+  static getInstance<T, R extends any[]>(this: StaticThis<T, R>, strict: false): T | undefined;
+  static getInstance<T, R extends any[]>(this: StaticThis<T, R>, strict = true): T | undefined {
+    if (!ExtensionStore.instances.has(this) && strict) {
+      throw new TypeError(`instance of ${this.name} is not created`);
+    }
+
+    return ExtensionStore.instances.get(this) as (T | undefined);
+  }
+
+  /**
+   * @deprecated This is a form of global shared state. Just call `new Store(...)`
+   */
+  static resetInstance() {
+    ExtensionStore.instances.delete(this);
+  }
+
   readonly displayName = "ExtensionStore<T>";
   protected extension?: LensExtension;
 

src/extensions/extensions-store/extensions-store.injectable.ts

@@ -7,14 +7,7 @@ import { ExtensionsStore } from "./extensions-store";
 
 const extensionsStoreInjectable = getInjectable({
   id: "extensions-store",
-
-  instantiate: () => {
-    ExtensionsStore.resetInstance();
-
-    return ExtensionsStore.createInstance();
-  },
-
-  causesSideEffects: true,
+  instantiate: () => new ExtensionsStore(),
 });
 
 export default extensionsStoreInjectable;

src/renderer/bootstrap.tsx

@@ -19,7 +19,6 @@ import { DefaultProps } from "./mui-base-theme";
 import configurePackages from "../common/configure-packages";
 import * as initializers from "./initializers";
 import logger from "../common/logger";
-import { WeblinkStore } from "../common/weblink-store";
 import { registerCustomThemes } from "./components/monaco-editor";
 import { getDi } from "./getDi";
 import { DiContextProvider } from "@ogre-tools/injectable-react";
@@ -130,8 +129,6 @@ export async function bootstrap(di: DiContainer) {
   // TODO: Remove temporal dependencies
   di.inject(themeStoreInjectable);
 
-  WeblinkStore.createInstance();
-
   const extensionInstallationStateStore = di.inject(extensionInstallationStateStoreInjectable);
 
   extensionInstallationStateStore.bindIpcListeners();