minor refactoring, added more unit-tests

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

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

@@ -1,4 +1,6 @@
+import { reaction } from "mobx";
 import { StorageAdapter, StorageHelper } from "../storageHelper";
+import { delay } from "../../../common/utils/delay";
 
 describe("renderer/utils/StorageHelper", () => {
   describe("window.localStorage might be used as StorageAdapter", () => {
@@ -32,13 +34,13 @@ describe("renderer/utils/StorageHelper", () => {
     });
 
     it("updates storage", async () => {
-      await storageHelper.init();
+      storageHelper.init();
 
       storageHelper.set("test2");
       expect(localStorage.getItem(storageKey)).toBe("test2");
 
       localStorage.setItem(storageKey, "test3");
-      await storageHelper.load(); // reload from underlying storage
+      storageHelper.init({ force: true }); // reload from underlying storage and merge
       expect(storageHelper.get()).toBe("test3");
     });
   });
@@ -52,6 +54,7 @@ describe("renderer/utils/StorageHelper", () => {
     const storageKey = "mySettings";
     const storageMock: Record<string, any> = {};
     let storageHelper: StorageHelper<SettingsStorageModel>;
+    let storageHelperAsync: StorageHelper<SettingsStorageModel>;
     let storageAdapter: StorageAdapter<SettingsStorageModel>;
 
     const storageHelperDefaultValue: SettingsStorageModel = {
@@ -66,7 +69,7 @@ describe("renderer/utils/StorageHelper", () => {
 
       storageAdapter = {
         onChange: jest.fn(),
-        getItem: jest.fn((key: string): Promise<any> => {
+        getItem: jest.fn((key: string) => {
           return storageMock[key];
         }),
         setItem: jest.fn((key: string, value: any) => {
@@ -79,30 +82,69 @@ describe("renderer/utils/StorageHelper", () => {
 
       storageHelper = new StorageHelper(storageKey, {
         autoInit: false,
-        storage: storageAdapter,
         defaultValue: storageHelperDefaultValue,
+        storage: storageAdapter,
+      });
+
+      storageHelperAsync = new StorageHelper(storageKey, {
+        autoInit: false,
+        defaultValue: storageHelperDefaultValue,
+        storage: {
+          ...storageAdapter,
+          async getItem(key: string): Promise<SettingsStorageModel> {
+            await delay(500); // fake loading timeout
+
+            return storageAdapter.getItem(key);
+          }
+        },
       });
     });
 
-    it("loads data from storage with fallback to default-value", async () => {
+    it("loads data from storage with fallback to default-value", () => {
       expect(storageHelper.get()).toEqual(storageHelperDefaultValue);
+      storageHelper.init();
 
-      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();
+    it("async loading from storage supported too", async () => {
+      expect(storageHelperAsync.initialized).toBeFalsy();
+      storageHelperAsync.init();
+      await delay(300);
+      expect(storageHelperAsync.get()).toEqual(storageHelperDefaultValue);
+      await delay(200);
+      expect(storageHelperAsync.get().message).toBe("saved-before");
+    });
 
+    it("set() fully replaces data in storage", () => {
+      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();
+    it("merge() does partial data tree updates", () => {
+      storageHelper.init();
+      storageHelper.merge({ message: "updated" });
+
+      expect(storageHelper.get()).toEqual({ ...storageHelperDefaultValue, message: "updated" });
+      expect(storageAdapter.setItem).toHaveBeenCalledWith(storageHelper.key, { ...storageHelperDefaultValue, message: "updated" });
+
+      storageHelper.merge(draft => {
+        draft.message = "updated2";
+      });
+      expect(storageHelper.get()).toEqual({ ...storageHelperDefaultValue, message: "updated2" });
+
+      storageHelper.merge(draft => ({
+        message: draft.message.replace("2", "3")
+      }));
+      expect(storageHelper.get()).toEqual({ ...storageHelperDefaultValue, message: "updated3" });
+    });
+
+    it("clears data in storage", () => {
+      storageHelper.init();
 
       expect(storageHelper.get()).toBeTruthy();
       storageHelper.clear();
@@ -111,22 +153,38 @@ describe("renderer/utils/StorageHelper", () => {
       expect(storageAdapter.removeItem).toHaveBeenCalledWith(storageHelper.key);
     });
 
-    it("merges data into storage", async () => {
-      expect(storageHelper.get()).toEqual(storageHelperDefaultValue);
+  });
 
-      await storageHelper.init();
+  describe("data in storage-helper is observable (mobx)", () => {
+    let storageHelper: StorageHelper<any>;
+    const defaultValue: any = { firstName: "Joe" };
+    const observedChanges: any[] = [];
 
-      storageHelper.merge(draft => {
-        draft.message = "updated";
+    beforeEach(() => {
+      observedChanges.length = 0;
+
+      storageHelper = new StorageHelper<typeof defaultValue>("some-key", {
+        autoInit: true,
+        defaultValue,
+        storage: {
+          getItem: jest.fn(),
+          setItem: jest.fn(),
+          removeItem: jest.fn(),
+        },
+      });
+    });
+
+    it("storage.get() is observable", () => {
+      expect(storageHelper.get()).toEqual(defaultValue);
+
+      reaction(() => storageHelper.toJS(), change => {
+        observedChanges.push(change);
       });
 
-      const expectedValue = { ...storageHelperDefaultValue, message: "updated" };
-
-      expect(storageHelper.get()).toEqual(expectedValue);
-      expect(storageAdapter.setItem).toHaveBeenCalledWith(storageHelper.key, expectedValue);
+      storageHelper.merge({ lastName: "Black" });
+      storageHelper.set("whatever");
+      expect(observedChanges).toEqual([{ ...defaultValue, lastName: "Black" }, "whatever",]);
     });
   });
 
-  // TODO: test observability options
-
 });

src/renderer/utils/storageHelper.ts

@@ -3,7 +3,8 @@
 import type { CreateObservableOptions } from "mobx/lib/api/observable";
 import { action, comparer, observable, toJS, when } from "mobx";
 import produce, { Draft, enableMapSet, setAutoFreeze } from "immer";
-import { isEqual, isFunction, isObject } from "lodash";
+import { isEqual, isFunction, isPlainObject } from "lodash";
+import logger from "../../main/logger";
 
 setAutoFreeze(false); // allow to merge observables
 enableMapSet(); // allow merging maps and sets
@@ -55,15 +56,43 @@ export class StorageHelper<T> {
   }
 
   @action
-  async init() {
-    if (this.initialized) return;
+  init({ force = false } = {}) {
+    if (this.initialized && !force) return;
+
+    this.loadFromStorage({
+      onData: (data: T) => {
+        const notEmpty = data != null;
+        const notDefault = !this.isDefaultValue(data);
+
+        if (notEmpty && notDefault) {
+          this.merge(data);
+        }
+
+        this.initialized = true;
+      },
+      onError: (error?: any) => {
+        logger.error(`[init]: ${error}`, this);
+      },
+    });
+  }
+
+  private loadFromStorage(opts: { onData?(data: T): void, onError?(error?: any): void } = {}) {
+    let data: T | Promise<T>;
 
     try {
-      await this.load();
-      this.initialized = true;
+      data = this.storage.getItem(this.key); // sync reading from storage when exposed
+
+      if (data instanceof Promise) {
+        data.then(opts.onData, opts.onError);
+      } else {
+        opts?.onData(data);
+      }
     } catch (error) {
-      console.error(`[init]: ${error}`, this);
+      logger.error(`[load]: ${error}`, this);
+      opts?.onError(error);
     }
+
+    return data;
   }
 
   isDefaultValue(value: T): boolean {
@@ -95,22 +124,10 @@ export class StorageHelper<T> {
 
       this.storage.onChange?.({ value, oldValue, key: this.key });
     } catch (error) {
-      console.error(`[change]: ${error}`, this, { value, oldValue });
+      logger.error(`[change]: ${error}`, this, { value, oldValue });
     }
   }
 
-  async load(): Promise<T> {
-    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 {
     return this.data.get();
   }
@@ -128,14 +145,15 @@ export class StorageHelper<T> {
   }
 
   merge(value: Partial<T> | ((draft: Draft<T>) => Partial<T> | void)) {
-    const updater = isFunction(value) ? value : (state: Draft<T>) => {
-      if (isObject(state)) return Object.assign(state, value);
+    const nextValue = produce(this.get(), (state: Draft<T>) => {
+      const newValue = isFunction(value) ? value(state) : value;
 
-      return value;
-    };
-    const nextValue = produce(this.get(), updater) as T;
+      return isPlainObject(newValue)
+        ? Object.assign(state, newValue) // partial updates for returned plain objects
+        : newValue;
+    });
 
-    this.set(nextValue);
+    this.set(nextValue as T);
   }
 
   toJS() {