impr: add fallback and migrate to parsejsonwithschema (@Miodec) (monkeytypegame#6518)

!nuf

Author: Miodec

frontend/__tests__/utils/local-storage-with-schema.spec.ts

@@ -68,14 +68,16 @@ describe("local-storage-with-schema.ts", () => {
       expect(res).toEqual(defaultObject);
     });
 
-    it("should revert to the fallback value and remove if localstorage json is malformed", () => {
+    it("should revert to the fallback value if localstorage json is malformed", () => {
       getItemMock.mockReturnValue("badjson");
 
       const res = ls.get();
 
       expect(localStorage.getItem).toHaveBeenCalledWith("config");
-      expect(localStorage.removeItem).toHaveBeenCalledWith("config");
-      expect(localStorage.setItem).not.toHaveBeenCalled();
+      expect(localStorage.setItem).toHaveBeenCalledWith(
+        "config",
+        JSON.stringify(defaultObject)
+      );
       expect(res).toEqual(defaultObject);
     });
 
@@ -132,8 +134,7 @@ describe("local-storage-with-schema.ts", () => {
       expect(localStorage.getItem).toHaveBeenCalledWith("config");
       expect(migrateFnMock).toHaveBeenCalledWith(
         existingValue,
-        expect.any(Array),
-        defaultObject
+        expect.any(Array)
       );
       expect(localStorage.setItem).toHaveBeenCalledWith(
         "config",
@@ -167,8 +168,7 @@ describe("local-storage-with-schema.ts", () => {
       expect(localStorage.getItem).toHaveBeenCalledWith("config");
       expect(migrateFnMock).toHaveBeenCalledWith(
         existingValue,
-        expect.any(Array),
-        defaultObject
+        expect.any(Array)
       );
       expect(localStorage.setItem).toHaveBeenCalledWith(
         "config",

frontend/__tests__/utils/url-handler.spec.ts

@@ -251,16 +251,7 @@ describe("url-handler", () => {
 
       //THEN
       expect(addNotificationMock).toHaveBeenCalledWith(
-        `Failed to load test settings from URL: \"0\" Invalid enum value. Expected 'time' | 'words' | 'quote' | 'custom' | 'zen', received 'invalidMode'
-\"1\" Needs to be a number or a number represented as a string e.g. \"10\".
-\"2.text\" Expected array, received string
-\"2.mode\" Invalid enum value. Expected 'repeat' | 'random' | 'shuffle', received 'invalid'
-\"2.limit\" Expected object, received string
-\"2.pipeDelimiter\" Expected boolean, received string
-\"3\" Expected boolean, received string
-\"4\" Expected boolean, received string
-\"6\" Invalid enum value. Expected 'normal' | 'expert' | 'master', received 'invalid'
-\"7\" Invalid input`,
+        `Failed to load test settings from URL: JSON does not match schema: \"0\" invalid enum value. expected 'time' | 'words' | 'quote' | 'custom' | 'zen', received 'invalidmode', \"1\" needs to be a number or a number represented as a string e.g. \"10\"., \"2.text\" expected array, received string, \"2.mode\" invalid enum value. expected 'repeat' | 'random' | 'shuffle', received 'invalid', \"2.limit\" expected object, received string, \"2.pipeDelimiter\" expected boolean, received string, \"3\" expected boolean, received string, \"4\" expected boolean, received string, \"6\" invalid enum value. expected 'normal' | 'expert' | 'master', received 'invalid', \"7\" invalid input`,
         0
       );
     });

frontend/src/ts/commandline/lists.ts

@@ -94,21 +94,17 @@ import * as Misc from "../utils/misc";
 import * as JSONData from "../utils/json-data";
 import { randomizeTheme } from "../controllers/theme-controller";
 import * as CustomTextPopup from "../modals/custom-text";
-import * as Settings from "../pages/settings";
 import * as Notifications from "../elements/notifications";
 import * as VideoAdPopup from "../popups/video-ad-popup";
 import * as ShareTestSettingsPopup from "../modals/share-test-settings";
 import * as TestStats from "../test/test-stats";
 import * as QuoteSearchModal from "../modals/quote-search";
 import * as FPSCounter from "../elements/fps-counter";
-import { migrateConfig } from "../utils/config";
 import {
   CustomBackgroundSchema,
   CustomLayoutFluid,
-  PartialConfigSchema,
 } from "@monkeytype/contracts/schemas/configs";
 import { Command, CommandsSubgroup } from "./types";
-import { parseWithSchema as parseJsonWithSchema } from "@monkeytype/util/json";
 import * as TestLogic from "../test/test-logic";
 import * as ActivePage from "../states/active-page";
 
@@ -378,21 +374,7 @@ export const commands: CommandsSubgroup = {
       input: true,
       exec: async ({ input }): Promise<void> => {
         if (input === undefined || input === "") return;
-        try {
-          const parsedConfig = parseJsonWithSchema(
-            input,
-            PartialConfigSchema.strip()
-          );
-          await UpdateConfig.apply(migrateConfig(parsedConfig));
-          UpdateConfig.saveFullConfigToLocalStorage();
-          void Settings.update();
-          Notifications.add("Done", 1);
-        } catch (e) {
-          Notifications.add(
-            "An error occured while importing settings: " + e,
-            -1
-          );
-        }
+        await UpdateConfig.applyFromJson(input);
       },
     },
     {

frontend/src/ts/config.ts

@@ -14,6 +14,7 @@ import {
   canSetFunboxWithConfig,
 } from "./test/funbox/funbox-validation";
 import {
+  createErrorMessage,
   isDevEnvironment,
   isObject,
   promiseWithResolvers,
@@ -28,6 +29,7 @@ import { LocalStorageWithSchema } from "./utils/local-storage-with-schema";
 import { migrateConfig } from "./utils/config";
 import { roundTo1 } from "@monkeytype/util/numbers";
 import { getDefaultConfig } from "./constants/default-config";
+import { parseWithSchema as parseJsonWithSchema } from "@monkeytype/util/json";
 
 const configLS = new LocalStorageWithSchema({
   key: "config",
@@ -2132,6 +2134,30 @@ export function getConfigChanges(): Partial<Config> {
   return configChanges;
 }
 
+export async function applyFromJson(json: string): Promise<void> {
+  try {
+    const parsedConfig = parseJsonWithSchema(
+      json,
+      ConfigSchemas.PartialConfigSchema.strip(),
+      {
+        migrate: (value) => {
+          if (Array.isArray(value)) {
+            throw new Error("Invalid config");
+          }
+          return migrateConfig(value);
+        },
+      }
+    );
+    await apply(parsedConfig);
+    saveFullConfigToLocalStorage();
+    Notifications.add("Done", 1);
+  } catch (e) {
+    const msg = createErrorMessage(e, "Failed to import settings");
+    console.error(msg);
+    Notifications.add(msg, -1);
+  }
+}
+
 const { promise: loadPromise, resolve: loadDone } = promiseWithResolvers();
 
 export { loadPromise };

frontend/src/ts/modals/import-export-settings.ts

@@ -1,9 +1,5 @@
-import { PartialConfigSchema } from "@monkeytype/contracts/schemas/configs";
 import * as UpdateConfig from "../config";
-import * as Notifications from "../elements/notifications";
 import AnimatedModal from "../utils/animated-modal";
-import { migrateConfig } from "../utils/config";
-import { parseWithSchema as parseJsonWithSchema } from "@monkeytype/util/json";
 
 type State = {
   mode: "import" | "export";
@@ -47,23 +43,7 @@ const modal = new AnimatedModal({
         void modal.hide();
         return;
       }
-      try {
-        const parsedConfig = parseJsonWithSchema(
-          state.value,
-          PartialConfigSchema.strip()
-        );
-        await UpdateConfig.apply(migrateConfig(parsedConfig));
-      } catch (e) {
-        Notifications.add(
-          "Failed to import settings: incorrect data schema",
-          0
-        );
-        console.error(e);
-        void modal.hide();
-        return;
-      }
-      Notifications.add("Settings imported", 1);
-      UpdateConfig.saveFullConfigToLocalStorage();
+      await UpdateConfig.applyFromJson(state.value);
       void modal.hide();
     });
   },

frontend/src/ts/test/custom-text.ts

@@ -7,6 +7,7 @@ import {
 import { LocalStorageWithSchema } from "../utils/local-storage-with-schema";
 import { z } from "zod";
 import { CustomTextDataWithTextLen } from "@monkeytype/contracts/schemas/results";
+import { deepClone } from "../utils/misc";
 
 const CustomTextObjectSchema = z.record(z.string(), z.string());
 type CustomTextObject = z.infer<typeof CustomTextObjectSchema>;
@@ -51,7 +52,9 @@ const customTextSettings = new LocalStorageWithSchema({
   key: "customTextSettings",
   schema: CustomTextSettingsSchema,
   fallback: defaultCustomTextSettings,
-  migrate: (oldData, _zodIssues, fallback) => {
+  migrate: (oldData, _zodIssues) => {
+    const fallback = deepClone(defaultCustomTextSettings);
+
     if (typeof oldData !== "object" || oldData === null) {
       return fallback;
     }
@@ -60,7 +63,7 @@ const customTextSettings = new LocalStorageWithSchema({
       "text" in oldData &&
       z.array(z.string()).safeParse(migratedData.text).success
     ) {
-      migratedData.text = oldData.text as string[];
+      migratedData.text = oldData["text"] as string[];
     }
     return migratedData;
   },

frontend/src/ts/utils/config.ts

@@ -14,6 +14,8 @@ import { getDefaultConfig } from "../constants/default-config";
  * @returns
  */
 export function migrateConfig(config: PartialConfig | object): Config {
+  //todo this assumes config is matching all schemas
+  //i think we should go through each value and validate
   return mergeWithDefaultConfig(replaceLegacyValues(config));
 }
 

frontend/src/ts/utils/local-storage-with-schema.ts

@@ -1,20 +1,26 @@
 import { ZodIssue } from "zod";
-import { deepClone } from "./misc";
 import { isZodError } from "@monkeytype/util/zod";
 import * as Notifications from "../elements/notifications";
 import { tryCatchSync } from "@monkeytype/util/trycatch";
+import { parseWithSchema as parseJsonWithSchema } from "@monkeytype/util/json";
 
 export class LocalStorageWithSchema<T> {
   private key: string;
   private schema: Zod.Schema<T>;
   private fallback: T;
-  private migrate?: (value: unknown, zodIssues: ZodIssue[], fallback: T) => T;
+  private migrate?: (
+    value: Record<string, unknown> | unknown[],
+    zodIssues?: ZodIssue[]
+  ) => T;
 
   constructor(options: {
     key: string;
     schema: Zod.Schema<T>;
     fallback: T;
-    migrate?: (value: unknown, zodIssues: ZodIssue[], fallback: T) => T;
+    migrate?: (
+      value: Record<string, unknown> | unknown[],
+      zodIssues?: ZodIssue[]
+    ) => T;
   }) {
     this.key = options.key;
     this.schema = options.schema;
@@ -29,49 +35,31 @@ export class LocalStorageWithSchema<T> {
       return this.fallback;
     }
 
-    const { data: jsonParsed, error } = tryCatchSync(
-      () => JSON.parse(value) as unknown
+    let migrated = false;
+    const { data: parsed, error } = tryCatchSync(() =>
+      parseJsonWithSchema(value, this.schema, {
+        fallback: this.fallback,
+        migrate: (oldData, zodIssues) => {
+          migrated = true;
+          if (this.migrate) {
+            return this.migrate(oldData, zodIssues);
+          } else {
+            return this.fallback;
+          }
+        },
+      })
     );
+
     if (error) {
-      console.log(
-        `Value from localStorage ${this.key} was not a valid JSON, using fallback`,
-        error
-      );
-      window.localStorage.removeItem(this.key);
+      window.localStorage.setItem(this.key, JSON.stringify(this.fallback));
       return this.fallback;
     }
 
-    const schemaParsed = this.schema.safeParse(jsonParsed);
-
-    if (schemaParsed.success) {
-      return schemaParsed.data;
-    }
-
-    console.log(
-      `Value from localStorage ${this.key} failed schema validation, migrating`,
-      schemaParsed.error.issues
-    );
-
-    let newValue = this.fallback;
-    if (this.migrate) {
-      const migrated = this.migrate(
-        jsonParsed,
-        schemaParsed.error.issues,
-        deepClone(this.fallback)
-      );
-      const parse = this.schema.safeParse(migrated);
-      if (parse.success) {
-        newValue = migrated;
-      } else {
-        console.error(
-          `Value from localStorage ${this.key} failed schema validation after migration! This is very bad!`,
-          parse.error.issues
-        );
-      }
+    if (migrated || parsed === this.fallback) {
+      window.localStorage.setItem(this.key, JSON.stringify(parsed));
     }
 
-    window.localStorage.setItem(this.key, JSON.stringify(newValue));
-    return newValue;
+    return parsed;
   }
 
   public set(data: T): boolean {