From 6fa0692bea7210879ef3048392b61f0a4507f179 Mon Sep 17 00:00:00 2001
From: Sebastian Malton <sebastian@malton.name>
Date: Wed, 18 Nov 2020 08:57:37 -0500
Subject: [PATCH] make BaseStore abstract so that implementors are forced to
 decide how to store data

Signed-off-by: Sebastian Malton <sebastian@malton.name>
---
 .../src/telemetry-preferences-store.ts        | 14 ++++-----
 src/common/base-store.ts                      | 30 +++++++++++--------
 2 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/extensions/telemetry/src/telemetry-preferences-store.ts b/extensions/telemetry/src/telemetry-preferences-store.ts
index 732ad91524..e29b0ff461 100644
--- a/extensions/telemetry/src/telemetry-preferences-store.ts
+++ b/extensions/telemetry/src/telemetry-preferences-store.ts
@@ -1,11 +1,13 @@
 import { Store } from "@k8slens/extensions";
 import { toJS } from "mobx"
 
-export type TelemetryPreferencesModel = {
+export type TelemetryPreferencesModel = {
   enabled: boolean;
 }
 
 export class TelemetryPreferencesStore extends Store.ExtensionStore<TelemetryPreferencesModel> {
+  enabled = true;
+
   private constructor() {
     super({
       configName: "preferences-store",
@@ -15,17 +17,13 @@ export class TelemetryPreferencesStore extends Store.ExtensionStore<TelemetryPre
     })
   }
 
-  get enabled() {
-    return this.data.enabled
-  }
-
-  set enabled(v: boolean) {
-    this.data.enabled = v
+  protected fromStore({ enabled }: TelemetryPreferencesModel): void {
+    this.enabled = enabled
   }
 
   toJSON(): TelemetryPreferencesModel {
     return toJS({
-      enabled: this.data.enabled
+      enabled: this.enabled
     }, {
       recurseEverything: true
     })
diff --git a/src/common/base-store.ts b/src/common/base-store.ts
index 17cc8d08e1..e9df396810 100644
--- a/src/common/base-store.ts
+++ b/src/common/base-store.ts
@@ -15,7 +15,10 @@ export interface BaseStoreParams<T = any> extends ConfOptions<T> {
   syncOptions?: IReactionOptions;
 }
 
-export class BaseStore<T = any> extends Singleton {
+/**
+ * Note: T should only contain base JSON serializable types.
+ */
+export abstract class BaseStore<T = any> extends Singleton {
   protected storeConfig: Config<T>;
   protected syncDisposers: Function[] = [];
 
@@ -146,16 +149,19 @@ export class BaseStore<T = any> extends Singleton {
     }
   }
 
-  @action
-  protected fromStore(data: T) {
-    if (!data) return;
-    this.data = data;
-  }
+  /**
+   * fromStore is called internally when a child class syncs with the file
+   * system.
+   * @param data the parsed information read from the stored JSON file
+   */
+  protected abstract fromStore(data: T): void;
 
-  // todo: use "serializr" ?
-  toJSON(): T {
-    return toJS(this.data, {
-      recurseEverything: true,
-    })
-  }
+  /**
+   * toJSON is called when syncing the store to the filesystem. It should
+   * produce a JSON serializable object representaion of the current state.
+   *
+   * It is recommended that a round trip is valid. Namely, calling
+   * `this.fromStore(this.toJSON())` shouldn't change the state.
+   */
+  abstract toJSON(): T;
 }