Fix hotbar migration of workspaces (#3178)

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

@@ -95,7 +95,7 @@ describe("empty config", () => {
 
     mockFs(mockOpts);
 
-    await ClusterStore.createInstance().load();
+    ClusterStore.createInstance();
   });
 
   afterEach(() => {
@@ -203,7 +203,7 @@ describe("config with existing clusters", () => {
 
     mockFs(mockOpts);
 
-    return ClusterStore.createInstance().load();
+    return ClusterStore.createInstance();
   });
 
   afterEach(() => {
@@ -285,7 +285,7 @@ users:
 
     mockFs(mockOpts);
 
-    return ClusterStore.createInstance().load();
+    return ClusterStore.createInstance();
   });
 
   afterEach(() => {
@@ -344,7 +344,7 @@ describe("pre 2.0 config with an existing cluster", () => {
 
     mockFs(mockOpts);
 
-    return ClusterStore.createInstance().load();
+    return ClusterStore.createInstance();
   });
 
   afterEach(() => {
@@ -414,7 +414,7 @@ describe("pre 2.6.0 config with a cluster that has arrays in auth config", () =>
 
     mockFs(mockOpts);
 
-    return ClusterStore.createInstance().load();
+    return ClusterStore.createInstance();
   });
 
   afterEach(() => {
@@ -456,7 +456,7 @@ describe("pre 2.6.0 config with a cluster icon", () => {
 
     mockFs(mockOpts);
 
-    return ClusterStore.createInstance().load();
+    return ClusterStore.createInstance();
   });
 
   afterEach(() => {
@@ -495,7 +495,7 @@ describe("for a pre 2.7.0-beta.0 config without a workspace", () => {
 
     mockFs(mockOpts);
 
-    return ClusterStore.createInstance().load();
+    return ClusterStore.createInstance();
   });
 
   afterEach(() => {
@@ -531,7 +531,7 @@ describe("pre 3.6.0-beta.1 config with an existing cluster", () => {
 
     mockFs(mockOpts);
 
-    return ClusterStore.createInstance().load();
+    return ClusterStore.createInstance();
   });
 
   afterEach(() => {

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

@@ -23,7 +23,7 @@ import mockFs from "mock-fs";
 import { ClusterStore } from "../cluster-store";
 import { HotbarStore } from "../hotbar-store";
 
-jest.mock("../../renderer/api/catalog-entity-registry", () => ({
+jest.mock("../../main/catalog/catalog-entity-registry", () => ({
   catalogEntityRegistry: {
     items: [
       {
@@ -39,7 +39,14 @@ jest.mock("../../renderer/api/catalog-entity-registry", () => ({
           name: "my_shiny_cluster",
           source: "remote"
         }
-      }
+      },
+      {
+        metadata: {
+          uid: "catalog-entity",
+          name: "Catalog",
+          source: "app"
+        },
+      },
     ]
   }
 }));
@@ -120,29 +127,31 @@ jest.mock("electron", () => {
 
 describe("HotbarStore", () => {
   beforeEach(() => {
-    ClusterStore.resetInstance();
+    mockFs({
+      "tmp": {
+        "lens-hotbar-store.json": JSON.stringify({})
+      }
+    });
     ClusterStore.createInstance();
-
-    HotbarStore.resetInstance();
-    mockFs({ tmp: { "lens-hotbar-store.json": "{}" } });
+    HotbarStore.createInstance();
   });
 
   afterEach(() => {
+    ClusterStore.resetInstance();
+    HotbarStore.resetInstance();
     mockFs.restore();
   });
 
   describe("load", () => {
     it("loads one hotbar by default", () => {
-      HotbarStore.createInstance().load();
       expect(HotbarStore.getInstance().hotbars.length).toEqual(1);
     });
   });
 
   describe("add", () => {
     it("adds a hotbar", () => {
-      const hotbarStore = HotbarStore.createInstance();
+      const hotbarStore = HotbarStore.getInstance();
 
-      hotbarStore.load();
       hotbarStore.add({ name: "hottest" });
       expect(hotbarStore.hotbars.length).toEqual(2);
     });
@@ -150,23 +159,20 @@ describe("HotbarStore", () => {
 
   describe("hotbar items", () => {
     it("initially creates 12 empty cells", () => {
-      const hotbarStore = HotbarStore.createInstance();
+      const hotbarStore = HotbarStore.getInstance();
 
-      hotbarStore.load();
       expect(hotbarStore.getActive().items.length).toEqual(12);
     });
 
     it("initially adds catalog entity as first item", () => {
-      const hotbarStore = HotbarStore.createInstance();
+      const hotbarStore = HotbarStore.getInstance();
 
-      hotbarStore.load();
       expect(hotbarStore.getActive().items[0].entity.name).toEqual("Catalog");
     });
 
     it("adds items", () => {
-      const hotbarStore = HotbarStore.createInstance();
+      const hotbarStore = HotbarStore.getInstance();
 
-      hotbarStore.load();
       hotbarStore.addToHotbar(testCluster);
       const items = hotbarStore.getActive().items.filter(Boolean);
 
@@ -174,9 +180,8 @@ describe("HotbarStore", () => {
     });
 
     it("removes items", () => {
-      const hotbarStore = HotbarStore.createInstance();
+      const hotbarStore = HotbarStore.getInstance();
 
-      hotbarStore.load();
       hotbarStore.addToHotbar(testCluster);
       hotbarStore.removeFromHotbar("test");
       hotbarStore.removeFromHotbar("catalog-entity");
@@ -186,9 +191,8 @@ describe("HotbarStore", () => {
     });
 
     it("does nothing if removing with invalid uid", () => {
-      const hotbarStore = HotbarStore.createInstance();
+      const hotbarStore = HotbarStore.getInstance();
 
-      hotbarStore.load();
       hotbarStore.addToHotbar(testCluster);
       hotbarStore.removeFromHotbar("invalid uid");
       const items = hotbarStore.getActive().items.filter(Boolean);
@@ -197,9 +201,8 @@ describe("HotbarStore", () => {
     });
 
     it("moves item to empty cell", () => {
-      const hotbarStore = HotbarStore.createInstance();
+      const hotbarStore = HotbarStore.getInstance();
 
-      hotbarStore.load();
       hotbarStore.addToHotbar(testCluster);
       hotbarStore.addToHotbar(minikubeCluster);
       hotbarStore.addToHotbar(awsCluster);
@@ -213,9 +216,8 @@ describe("HotbarStore", () => {
     });
 
     it("moves items down", () => {
-      const hotbarStore = HotbarStore.createInstance();
+      const hotbarStore = HotbarStore.getInstance();
 
-      hotbarStore.load();
       hotbarStore.addToHotbar(testCluster);
       hotbarStore.addToHotbar(minikubeCluster);
       hotbarStore.addToHotbar(awsCluster);
@@ -229,9 +231,8 @@ describe("HotbarStore", () => {
     });
 
     it("moves items up", () => {
-      const hotbarStore = HotbarStore.createInstance();
+      const hotbarStore = HotbarStore.getInstance();
 
-      hotbarStore.load();
       hotbarStore.addToHotbar(testCluster);
       hotbarStore.addToHotbar(minikubeCluster);
       hotbarStore.addToHotbar(awsCluster);
@@ -245,9 +246,8 @@ describe("HotbarStore", () => {
     });
 
     it("does nothing when item moved to same cell", () => {
-      const hotbarStore = HotbarStore.createInstance();
+      const hotbarStore = HotbarStore.getInstance();
 
-      hotbarStore.load();
       hotbarStore.addToHotbar(testCluster);
       hotbarStore.restackItems(1, 1);
 
@@ -255,9 +255,8 @@ describe("HotbarStore", () => {
     });
 
     it("new items takes first empty cell", () => {
-      const hotbarStore = HotbarStore.createInstance();
+      const hotbarStore = HotbarStore.getInstance();
 
-      hotbarStore.load();
       hotbarStore.addToHotbar(testCluster);
       hotbarStore.addToHotbar(awsCluster);
       hotbarStore.restackItems(0, 3);
@@ -268,13 +267,13 @@ describe("HotbarStore", () => {
 
     it("throws if invalid arguments provided", () => {
       // Prevent writing to stderr during this render.
-      const err = console.error;
+      const { error, warn } = console;
 
       console.error = jest.fn();
+      console.warn = jest.fn();
 
-      const hotbarStore = HotbarStore.createInstance();
+      const hotbarStore = HotbarStore.getInstance();
 
-      hotbarStore.load();
       hotbarStore.addToHotbar(testCluster);
 
       expect(() => hotbarStore.restackItems(-5, 0)).toThrow();
@@ -283,7 +282,8 @@ describe("HotbarStore", () => {
       expect(() => hotbarStore.restackItems(11, 112)).toThrow();
 
       // Restore writing to stderr.
-      console.error = err;
+      console.error = error;
+      console.warn = warn;
     });
   });
 
@@ -354,7 +354,7 @@ describe("HotbarStore", () => {
 
       mockFs(mockOpts);
 
-      return HotbarStore.createInstance().load();
+      HotbarStore.createInstance();
     });
 
     afterEach(() => {

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

@@ -52,7 +52,7 @@ describe("user store tests", () => {
 
       (UserStore.createInstance() as any).refreshNewContexts = jest.fn(() => Promise.resolve());
 
-      return UserStore.getInstance().load();
+      UserStore.getInstance();
     });
 
     afterEach(() => {
@@ -81,10 +81,8 @@ describe("user store tests", () => {
     it("correctly resets theme to default value", async () => {
       const us = UserStore.getInstance();
 
-      us.isLoaded = true;
-
       us.colorTheme = "some other theme";
-      await us.resetTheme();
+      us.resetTheme();
       expect(us.colorTheme).toBe(UserStore.defaultTheme);
     });
 
@@ -111,7 +109,7 @@ describe("user store tests", () => {
         }
       });
 
-      return UserStore.createInstance().load();
+      UserStore.createInstance();
     });
 
     afterEach(() => {

src/common/base-store.ts

@@ -23,41 +23,42 @@ import path from "path";
 import Config from "conf";
 import type { Options as ConfOptions } from "conf/dist/source/types";
 import { app, ipcMain, ipcRenderer, remote } from "electron";
-import { IReactionOptions, makeObservable, observable, reaction, runInAction, when } from "mobx";
+import { IReactionOptions, makeObservable, reaction, runInAction } from "mobx";
 import { getAppVersion, Singleton, toJS, Disposer } from "./utils";
 import logger from "../main/logger";
 import { broadcastMessage, ipcMainOn, ipcRendererOn } from "./ipc";
 import isEqual from "lodash/isEqual";
 
-export interface BaseStoreParams<T = any> extends ConfOptions<T> {
-  autoLoad?: boolean;
-  syncEnabled?: boolean;
+export interface BaseStoreParams<T> extends ConfOptions<T> {
   syncOptions?: IReactionOptions;
 }
 
 /**
  * Note: T should only contain base JSON serializable types.
  */
-export abstract class BaseStore<T = any> extends Singleton {
+export abstract class BaseStore<T> extends Singleton {
   protected storeConfig?: Config<T>;
   protected syncDisposers: Disposer[] = [];
 
-  @observable isLoaded = false;
-
-  get whenLoaded() {
-    return when(() => this.isLoaded);
-  }
-
-  protected constructor(protected params: BaseStoreParams) {
+  protected constructor(protected params: BaseStoreParams<T>) {
     super();
     makeObservable(this);
+  }
 
-    this.params = {
-      autoLoad: false,
-      syncEnabled: true,
-      ...params,
-    };
-    this.init();
+  /**
+   * This must be called after the last child's constructor is finished (or just before it finishes)
+   */
+  load() {
+    this.storeConfig = new Config({
+      ...this.params,
+      projectName: "lens",
+      projectVersion: getAppVersion(),
+      cwd: this.cwd(),
+    });
+
+    logger.info(`[STORE]: LOADED from ${this.path}`);
+    this.fromStore(this.storeConfig.store);
+    this.enableSync();
   }
 
   get name() {
@@ -76,31 +77,6 @@ export abstract class BaseStore<T = any> extends Singleton {
     return this.storeConfig?.path || "";
   }
 
-  protected async init() {
-    if (this.params.autoLoad) {
-      await this.load();
-    }
-
-    if (this.params.syncEnabled) {
-      await this.whenLoaded;
-      this.enableSync();
-    }
-  }
-
-  async load() {
-    const { autoLoad, syncEnabled, ...confOptions } = this.params;
-
-    this.storeConfig = new Config({
-      ...confOptions,
-      projectName: "lens",
-      projectVersion: getAppVersion(),
-      cwd: this.cwd(),
-    });
-    logger.info(`[STORE]: LOADED from ${this.path}`);
-    this.fromStore(this.storeConfig.store);
-    this.isLoaded = true;
-  }
-
   protected cwd() {
     return (app || remote.app).getPath("userData");
   }
@@ -159,10 +135,7 @@ export abstract class BaseStore<T = any> extends Singleton {
   protected applyWithoutSync(callback: () => void) {
     this.disableSync();
     runInAction(callback);
-
-    if (this.params.syncEnabled) {
-      this.enableSync();
-    }
+    this.enableSync();
   }
 
   protected onSync(model: T) {

src/common/cluster-store.ts

@@ -110,6 +110,8 @@ export interface ClusterPrometheusPreferences {
   };
 }
 
+const initialStates = "cluster:states";
+
 export class ClusterStore extends BaseStore<ClusterStoreModel> {
   private static StateChannel = "cluster:state";
 
@@ -121,8 +123,8 @@ export class ClusterStore extends BaseStore<ClusterStoreModel> {
     return path.resolve(ClusterStore.storedKubeConfigFolder, clusterId);
   }
 
-  @observable clusters = observable.map<ClusterId, Cluster>();
-  @observable removedClusters = observable.map<ClusterId, Cluster>();
+  clusters = observable.map<ClusterId, Cluster>();
+  removedClusters = observable.map<ClusterId, Cluster>();
 
   protected disposer = disposer();
 
@@ -137,31 +139,27 @@ export class ClusterStore extends BaseStore<ClusterStoreModel> {
     });
 
     makeObservable(this);
-
+    this.load();
     this.pushStateToViewsAutomatically();
   }
 
-  async load() {
-    const initialStates = "cluster:states";
+  async loadInitialOnRenderer() {
+    logger.info("[CLUSTER-STORE] requesting initial state sync");
 
-    await super.load();
-
-    if (ipcRenderer) {
-      logger.info("[CLUSTER-STORE] requesting initial state sync");
-
-      for (const { id, state } of await requestMain(initialStates)) {
-        this.getById(id)?.setState(state);
-      }
-    } else if (ipcMain) {
-      ipcMainHandle(initialStates, () => {
-        return this.clustersList.map(cluster => ({
-          id: cluster.id,
-          state: cluster.getState(),
-        }));
-      });
+    for (const { id, state } of await requestMain(initialStates)) {
+      this.getById(id)?.setState(state);
     }
   }
 
+  provideInitialFromMain() {
+    ipcMainHandle(initialStates, () => {
+      return this.clustersList.map(cluster => ({
+        id: cluster.id,
+        state: cluster.getState(),
+      }));
+    });
+  }
+
   protected pushStateToViewsAutomatically() {
     if (ipcMain) {
       this.disposer.push(

src/common/hotbar-store.ts

@@ -26,7 +26,6 @@ import * as uuid from "uuid";
 import isNull from "lodash/isNull";
 import { toJS } from "./utils";
 import { CatalogEntity } from "./catalog";
-import { catalogEntity } from "../main/catalog-sources/general";
 
 export interface HotbarItem {
   entity: {
@@ -39,16 +38,12 @@ export interface HotbarItem {
   }
 }
 
-export interface Hotbar {
-  id: string;
-  name: string;
-  items: HotbarItem[];
-}
+export type Hotbar = Required<HotbarCreateOptions>;
 
 export interface HotbarCreateOptions {
   id?: string;
   name: string;
-  items?: HotbarItem[];
+  items?: (HotbarItem | null)[];
 }
 
 export interface HotbarStoreModel {
@@ -72,6 +67,7 @@ export class HotbarStore extends BaseStore<HotbarStoreModel> {
       migrations,
     });
     makeObservable(this);
+    this.load();
   }
 
   get activeHotbarId() {
@@ -92,30 +88,13 @@ export class HotbarStore extends BaseStore<HotbarStoreModel> {
     return this.hotbarIndex(this.activeHotbarId);
   }
 
-  get defaultHotbarInitialItems() {
-    const { metadata: { uid, name, source } } = catalogEntity;
-    const initialItem = { entity: { uid, name, source }};
-
-    return [
-      initialItem,
-      ...Array.from(Array(defaultHotbarCells - 1).fill(null))
-    ];
-  }
-
-  get initialItems() {
+  static getInitialItems() {
     return [...Array.from(Array(defaultHotbarCells).fill(null))];
   }
 
-  @action protected async fromStore(data: Partial<HotbarStoreModel> = {}) {
-    if (data.hotbars?.length === 0) {
-      this.hotbars = [{
-        id: uuid.v4(),
-        name: "Default",
-        items: this.defaultHotbarInitialItems,
-      }];
-    } else {
-      this.hotbars = data.hotbars;
-    }
+  @action
+  protected async fromStore(data: Partial<HotbarStoreModel> = {}) {
+    this.hotbars = data.hotbars;
 
     if (data.activeHotbarId) {
       if (this.getById(data.activeHotbarId)) {
@@ -140,18 +119,19 @@ export class HotbarStore extends BaseStore<HotbarStoreModel> {
     return this.hotbars.find((hotbar) => hotbar.id === id);
   }
 
-  add(data: HotbarCreateOptions) {
+  @action
+  add(data: HotbarCreateOptions, { setActive = false } = {}) {
     const {
       id = uuid.v4(),
-      items = this.initialItems,
+      items = HotbarStore.getInitialItems(),
       name,
     } = data;
 
-    const hotbar = { id, name, items };
+    this.hotbars.push({ id, name, items });
 
-    this.hotbars.push(hotbar as Hotbar);
-
-    return hotbar as Hotbar;
+    if (setActive) {
+      this._activeHotbarId = id;
+    }
   }
 
   @action

src/common/user-store.ts

@@ -68,7 +68,10 @@ export class UserStore extends BaseStore<UserStoreModel> {
       configName: "lens-user-store",
       migrations,
     });
+
     makeObservable(this);
+    fileNameMigration();
+    this.load();
   }
 
   @observable lastSeenAppVersion = "0.0.0";
@@ -101,33 +104,6 @@ export class UserStore extends BaseStore<UserStoreModel> {
     [path.join(os.homedir(), ".kube"), {}]
   ]);
 
-  async load(): Promise<void> {
-    /**
-     * This has to be here before the call to `new Config` in `super.load()`
-     * as we have to make sure that file is in the expected place for that call
-     */
-    await fileNameMigration();
-    await super.load();
-
-    if (app) {
-      // track telemetry availability
-      reaction(() => this.allowTelemetry, allowed => {
-        appEventBus.emit({ name: "telemetry", action: allowed ? "enabled" : "disabled" });
-      });
-
-      // open at system start-up
-      reaction(() => this.openAtLogin, openAtLogin => {
-        app.setLoginItemSettings({
-          openAtLogin,
-          openAsHidden: true,
-          args: ["--hidden"]
-        });
-      }, {
-        fireImmediately: true,
-      });
-    }
-  }
-
   @computed get isNewVersion() {
     return semver.gt(getAppVersion(), this.lastSeenAppVersion);
   }
@@ -136,6 +112,24 @@ export class UserStore extends BaseStore<UserStoreModel> {
     return this.shell || process.env.SHELL || process.env.PTYSHELL;
   }
 
+  startMainReactions() {
+    // track telemetry availability
+    reaction(() => this.allowTelemetry, allowed => {
+      appEventBus.emit({ name: "telemetry", action: allowed ? "enabled" : "disabled" });
+    });
+
+    // open at system start-up
+    reaction(() => this.openAtLogin, openAtLogin => {
+      app.setLoginItemSettings({
+        openAtLogin,
+        openAsHidden: true,
+        args: ["--hidden"]
+      });
+    }, {
+      fireImmediately: true,
+    });
+  }
+
   /**
    * Checks if a column (by ID) for a table (by ID) is configured to be hidden
    * @param tableId The ID of the table to be checked against
@@ -165,8 +159,7 @@ export class UserStore extends BaseStore<UserStoreModel> {
   }
 
   @action
-  async resetTheme() {
-    await this.whenLoaded;
+  resetTheme() {
     this.colorTheme = UserStore.defaultTheme;
   }
 

src/common/weblink-store.ts

@@ -55,6 +55,7 @@ export class WeblinkStore extends BaseStore<WeblinkStoreModel> {
       migrations,
     });
     makeObservable(this);
+    this.load();
   }
 
   @action protected async fromStore(data: Partial<WeblinkStoreModel> = {}) {