refactoring, adding some unit tests

Signed-off-by: Roman <ixrock@gmail.com>

src/renderer/hooks/useStorage.ts

@@ -1,7 +1,8 @@
 import { useState } from "react";
-import { createStorage, StorageHelperOptions } from "../utils";
+import { createStorage } from "../utils";
+import { CreateObservableOptions } from "mobx/lib/api/observable";
 
-export function useStorage<T>(key: string, initialValue?: T, options?: StorageHelperOptions<T>) {
+export function useStorage<T>(key: string, initialValue?: T, options?: CreateObservableOptions) {
   const storage = createStorage(key, initialValue, options);
   const [storageValue, setStorageValue] = useState(storage.get());
   const setValue = (value: T) => {

src/renderer/utils/__tests__/createStorage.test.ts

@@ -0,0 +1,130 @@
+import { StorageAdapter, StorageHelper } from "../createStorage";
+
+describe("renderer/utils/StorageHelper", () => {
+  describe("window.localStorage might be used as StorageAdapter", () => {
+    type StorageModel = string;
+
+    const storageKey = "ui-settings";
+    let storageHelper: StorageHelper<StorageModel>;
+
+    beforeEach(() => {
+      localStorage.clear();
+
+      storageHelper = new StorageHelper<StorageModel>(storageKey, {
+        autoInit: false,
+        storage: localStorage,
+        defaultValue: "test",
+      });
+    });
+
+    it("initialized with default value", async () => {
+      localStorage.setItem(storageKey, "saved"); // pretending it was saved previously
+
+      expect(storageHelper.key).toBe(storageKey);
+      expect(storageHelper.defaultValue).toBe("test");
+      expect(storageHelper.get()).toBe("test");
+
+      await storageHelper.init();
+
+      expect(storageHelper.key).toBe(storageKey);
+      expect(storageHelper.defaultValue).toBe("test");
+      expect(storageHelper.get()).toBe("saved");
+    });
+
+    it("updates storage", async () => {
+      await storageHelper.init();
+
+      storageHelper.set("test2");
+      expect(localStorage.getItem(storageKey)).toBe("test2");
+
+      localStorage.setItem(storageKey, "test3");
+      await storageHelper.load(); // reload from underlying storage
+      expect(storageHelper.get()).toBe("test3");
+    });
+  });
+
+  describe("Using custom StorageAdapter", () => {
+    type SettingsStorageModel = {
+      [key: string]: any;
+      message: string;
+    };
+
+    const storageKey = "mySettings";
+    const storageMock: Record<string, any> = {};
+    let storageHelper: StorageHelper<SettingsStorageModel>;
+    let storageAdapter: StorageAdapter<SettingsStorageModel>;
+
+    const storageHelperDefaultValue: SettingsStorageModel = {
+      message: "hello-world",
+      anyOtherStorableData: 123,
+    };
+
+    beforeEach(() => {
+      storageMock[storageKey] = {
+        message: "saved-before",
+      } as SettingsStorageModel;
+
+      storageAdapter = {
+        onChange: jest.fn(),
+        getItem: jest.fn((key: string): Promise<any> => {
+          return storageMock[key];
+        }),
+        setItem: jest.fn((key: string, value: any) => {
+          storageMock[key] = value;
+        }),
+        removeItem: jest.fn((key: string) => {
+          delete storageMock[key];
+        }),
+      };
+
+      storageHelper = new StorageHelper(storageKey, {
+        autoInit: false,
+        storage: storageAdapter,
+        defaultValue: storageHelperDefaultValue,
+      });
+    });
+
+    it("loads data from storage with fallback to default-value", async () => {
+      expect(storageHelper.get()).toEqual(storageHelperDefaultValue);
+
+      await storageHelper.init();
+      expect(storageHelper.get().message).toBe("saved-before");
+      expect(storageAdapter.getItem).toHaveBeenCalledWith(storageHelper.key);
+    });
+
+    it("updates data in storage", async () => {
+      await storageHelper.init();
+
+      storageHelper.set({ message: "test2" });
+      expect(storageHelper.get().message).toBe("test2");
+      expect(storageMock[storageKey]).toEqual({ message: "test2" });
+      expect(storageAdapter.setItem).toHaveBeenCalledWith(storageHelper.key, { message: "test2" });
+    });
+
+    it("deletes data in storage", async () => {
+      await storageHelper.init();
+
+      expect(storageHelper.get()).toBeTruthy();
+      storageHelper.clear();
+      expect(storageHelper.get()).toBeFalsy();
+      expect(storageMock[storageKey]).toBeUndefined();
+      expect(storageAdapter.removeItem).toHaveBeenCalledWith(storageHelper.key);
+    });
+
+    it("merges data into storage", async () => {
+      expect(storageHelper.get()).toEqual(storageHelperDefaultValue);
+
+      await storageHelper.init();
+
+      storageHelper.merge(draft => {
+        draft.message = "updated";
+      });
+
+      const expectedValue = { ...storageHelperDefaultValue, message: "updated" };
+
+      expect(storageHelper.get()).toEqual(expectedValue);
+      expect(storageAdapter.setItem).toHaveBeenCalledWith(storageHelper.key, expectedValue);
+    });
+  });
+
+});

src/renderer/utils/createStorage.ts

@@ -1,57 +1,66 @@
 // Helper for work with persistent local storage (default: window.localStorage)
-// TODO: write unit/integration tests
 
 import { app, remote } from "electron";
 import path from "path";
 import { ensureDir, readJson, writeJson } from "fs-extra";
 import type { CreateObservableOptions } from "mobx/lib/api/observable";
 import { action, comparer, observable, reaction, toJS, when } from "mobx";
-import produce, { Draft, setAutoFreeze } from "immer";
+import produce, { Draft, enableMapSet, setAutoFreeze } from "immer";
 import { isEqual, isFunction, isObject, noop } from "lodash";
 
 setAutoFreeze(false); // allow to merge observables
+enableMapSet(); // allow merging maps and sets
 
-export function createStorage<T>(key: string, defaultValue?: T, options?: StorageHelperOptions<T>) {
-  return new StorageHelper(key, defaultValue, options);
+export function createStorage<T>(key: string, defaultValue?: T, observableOptions?: CreateObservableOptions) {
+  return new StorageHelper<T>(key, {
+    autoInit: true,
+    storage: jsonFileStorageAdapter as StorageAdapter<T>,
+    observable: observableOptions,
+    defaultValue,
+  });
 }
 
-export interface StorageHelperOptions<T> extends StorageConfiguration<T> {
-  autoInit?: boolean; // default: true
-}
+export type StorageModel = Record<string, any>; // any json-compatible model
 
-export interface StorageConfiguration<T> {
-  storage?: StorageAdapter<T>;
-  observable?: CreateObservableOptions;
-}
-
-export interface StorageAdapter<T> {
+export interface StorageAdapter<T = StorageModel> {
   [metadata: string]: any;
-  getItem(key: string): T | Promise<T>; // import
-  setItem(key: string, value: T): void; // export
-  removeItem?(key: string): void; // if not provided setItem(key,undefined) will be used
+  getItem(key: string): T | Promise<T>;
+  setItem(key: string, value: T): void;
+  removeItem(key: string): void;
   onChange?(change: { key: string, value: T, oldValue?: T }): void;
 }
 
-export class StorageHelper<T> {
-  static defaultOptions: StorageHelperOptions<any> = {
+export interface StorageHelperOptions<T> {
+  autoInit?: boolean; // start preloading data immediately, default: true
+  observable?: CreateObservableOptions;
+  storage: StorageAdapter<T>;
+  defaultValue?: T;
+}
+
+export class StorageHelper<T = StorageModel> {
+  static defaultOptions: Partial<StorageHelperOptions<any>> = {
     autoInit: true,
-    get storage() {
-      return jsonFileStorageAdapter;
-    },
     observable: {
       deep: true,
       equals: comparer.shallow,
     }
   };
 
-  private data = observable.box<T>();
-  @observable.ref storage: StorageAdapter<T>;
+  @observable private data = observable.box<T>();
   @observable initialized = false;
   whenReady = when(() => this.initialized);
 
-  constructor(readonly key: string, readonly defaultValue?: T, readonly options: StorageHelperOptions<T> = {}) {
+  get storage(): StorageAdapter<T> {
+    return this.options.storage;
+  }
+
+  get defaultValue(): T {
+    return this.options.defaultValue;
+  }
+
+  constructor(readonly key: string, readonly options: StorageHelperOptions<T>) {
     this.options = { ...StorageHelper.defaultOptions, ...options };
-    this.configure();
+    this.configureObservable();
     this.reset();
 
     if (this.options.autoInit) {
@@ -64,13 +73,7 @@ export class StorageHelper<T> {
     if (this.initialized) return;
 
     try {
-      const value = await this.load();
-      const notEmpty = value != null;
-      const notDefault = !this.isDefaultValue(value);
-
-      if (notEmpty && notDefault) {
-        this.merge(value);
-      }
+      await this.load();
       this.initialized = true;
     } catch (error) {
       console.error(`[init]: ${error}`, this);
@@ -82,18 +85,10 @@ export class StorageHelper<T> {
   }
 
   @action
-  private configure({ storage, observable }: StorageConfiguration<T> = this.options): this {
-    if (storage) this.storage = storage;
-    if (observable) this.configureObservable(observable);
-
-    return this;
-  }
-
-  @action
-  private configureObservable(options: CreateObservableOptions = {}) {
+  private configureObservable(options = this.options.observable) {
     this.data = observable.box<T>(this.data.get(), {
       ...StorageHelper.defaultOptions.observable, // inherit default observability options
-      ...options,
+      ...(options ?? {}),
     });
     this.data.observe(change => {
       const { newValue, oldValue } = toJS(change, { recurseEverything: true });
@@ -105,12 +100,29 @@ export class StorageHelper<T> {
   protected onChange(value: T, oldValue?: T) {
     if (!this.initialized) return;
 
-    this.storage.onChange?.({ value, oldValue, key: this.key });
-    this.storage.setItem(this.key, value);
+    try {
+      if (value == null) {
+        this.storage.removeItem(this.key);
+      } else {
+        this.storage.setItem(this.key, value);
+      }
+
+      this.storage.onChange?.({ value, oldValue, key: this.key });
+    } catch (error) {
+      console.error(`[change]: ${error}`, this, { value, oldValue });
+    }
   }
 
   async load(): Promise<T> {
-    return this.storage.getItem(this.key);
+    const value = await this.storage.getItem(this.key);
+    const notEmpty = value != null;
+    const notDefault = !this.isDefaultValue(value);
+
+    if (notEmpty && notDefault) {
+      this.merge(value);
+    }
+
+    return value;
   }
 
   get(): T {
@@ -146,28 +158,17 @@ export class StorageHelper<T> {
   }
 }
 
-export const localStorageAdapter: StorageAdapter<object> = {
-  getItem(key: string) {
-    return JSON.parse(localStorage.getItem(key));
-  },
-  setItem(key: string, value: any) {
-    localStorage.setItem(key, JSON.stringify(value));
-  },
-  removeItem(key: string) {
-    localStorage.removeItem(key);
-  }
-};
-
 /**
  * Keep intended window.localStorage state in external JSON-file.
  * Reason: app creates random ports between restarts and as a result storage not persistent.
  */
-export const jsonFileStorageAdapter: StorageAdapter<object> = {
+export const jsonFileStorageAdapter: StorageAdapter = {
   cwd: path.resolve((app || remote.app).getPath("userData"), "lens-local-storage"),
   data: observable.map<string, any>([], { deep: false }),
   initialized: false,
 
   get filePath() {
+    // TODO: use getHostedClusterId() after moving out from cluster-store.ts
     const clusterId = location.hostname.split(".").slice(-2, -1)[0];
 
     return path.resolve(this.cwd, `${clusterId ?? "app"}.json`);