diff --git a/frontend/__tests__/utils/local-storage-with-schema.spec.ts b/frontend/__tests__/utils/local-storage-with-schema.spec.ts index 08a6e7df2..fa74cf040 100644 --- a/frontend/__tests__/utils/local-storage-with-schema.spec.ts +++ b/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", diff --git a/frontend/__tests__/utils/url-handler.spec.ts b/frontend/__tests__/utils/url-handler.spec.ts index 13d948f3f..744377be9 100644 --- a/frontend/__tests__/utils/url-handler.spec.ts +++ b/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 ); }); diff --git a/frontend/src/ts/commandline/lists.ts b/frontend/src/ts/commandline/lists.ts index 04519bfaf..1766d685a 100644 --- a/frontend/src/ts/commandline/lists.ts +++ b/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 => { 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); }, }, { diff --git a/frontend/src/ts/config.ts b/frontend/src/ts/config.ts index 79272a35a..5aed009ce 100644 --- a/frontend/src/ts/config.ts +++ b/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 { return configChanges; } +export async function applyFromJson(json: string): Promise { + 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 }; diff --git a/frontend/src/ts/modals/import-export-settings.ts b/frontend/src/ts/modals/import-export-settings.ts index 1994064dc..17717f916 100644 --- a/frontend/src/ts/modals/import-export-settings.ts +++ b/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(); }); }, diff --git a/frontend/src/ts/test/custom-text.ts b/frontend/src/ts/test/custom-text.ts index 7772aa8f3..bc5cdb84f 100644 --- a/frontend/src/ts/test/custom-text.ts +++ b/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; @@ -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; }, diff --git a/frontend/src/ts/utils/config.ts b/frontend/src/ts/utils/config.ts index 253af4a4b..c7b09f200 100644 --- a/frontend/src/ts/utils/config.ts +++ b/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)); } diff --git a/frontend/src/ts/utils/local-storage-with-schema.ts b/frontend/src/ts/utils/local-storage-with-schema.ts index 45c672d4b..01e8611a9 100644 --- a/frontend/src/ts/utils/local-storage-with-schema.ts +++ b/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 { private key: string; private schema: Zod.Schema; private fallback: T; - private migrate?: (value: unknown, zodIssues: ZodIssue[], fallback: T) => T; + private migrate?: ( + value: Record | unknown[], + zodIssues?: ZodIssue[] + ) => T; constructor(options: { key: string; schema: Zod.Schema; fallback: T; - migrate?: (value: unknown, zodIssues: ZodIssue[], fallback: T) => T; + migrate?: ( + value: Record | unknown[], + zodIssues?: ZodIssue[] + ) => T; }) { this.key = options.key; this.schema = options.schema; @@ -29,49 +35,31 @@ export class LocalStorageWithSchema { 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; + if (migrated || parsed === this.fallback) { + window.localStorage.setItem(this.key, JSON.stringify(parsed)); } - 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 - ); - } - } - - window.localStorage.setItem(this.key, JSON.stringify(newValue)); - return newValue; + return parsed; } public set(data: T): boolean { diff --git a/packages/util/__test__/json.spec.ts b/packages/util/__test__/json.spec.ts index 008437a8f..2de9c4568 100644 --- a/packages/util/__test__/json.spec.ts +++ b/packages/util/__test__/json.spec.ts @@ -8,6 +8,13 @@ describe("json", () => { name: z.string(), nested: z.object({ foo: z.string() }).strict().optional(), }); + it("should throw with invalid json", () => { + expect(() => parseWithSchema("blah", schema)).toThrowError( + new Error( + `Invalid JSON: Unexpected token 'b', "blah" is not valid JSON` + ) + ); + }); it("should parse", () => { const json = `{ "test":true, @@ -24,7 +31,7 @@ describe("json", () => { nested: { foo: "bar" }, }); }); - it("should fail with invalid schema", () => { + it("should throw with invalid schema", () => { const json = `{ "test":"yes", "nested":{ @@ -34,9 +41,106 @@ describe("json", () => { expect(() => parseWithSchema(json, schema)).toThrowError( new Error( - `"test" Expected boolean, received string\n"name" Required\n"nested.foo" Expected string, received number` + `JSON does not match schema: "test" expected boolean, received string, "name" required, "nested.foo" expected string, received number` ) ); }); + it("should migrate if valid json", () => { + const json = `{ + "name": 1 + }`; + + const result = parseWithSchema(json, schema, { + migrate: () => { + return { + name: "migrated", + test: false, + }; + }, + }); + + expect(result).toStrictEqual({ + name: "migrated", + test: false, + }); + }); + it("should revert to fallback if invalid json", () => { + const json = `blah`; + + const result = parseWithSchema(json, schema, { + fallback: { + name: "migrated", + test: false, + }, + }); + + expect(result).toStrictEqual({ + name: "migrated", + test: false, + }); + }); + it("should throw if migration fails", () => { + const json = `{ + "name": 1 + }`; + + expect(() => { + parseWithSchema(json, schema, { + //@ts-expect-error need to test migration failure + migrate: () => { + return { + name: null, + test: "Hi", + }; + }, + }); + }).toThrowError( + new Error( + `Migrated value does not match schema: "test" expected boolean, received string, "name" expected string, received null` + ) + ); + }); + it("should revert to fallback if migration fails", () => { + const json = `{ + "name": 1 + }`; + + const result = parseWithSchema(json, schema, { + fallback: { + name: "fallback", + test: false, + }, + //@ts-expect-error need to test migration failure + migrate: () => { + return { + name: null, + test: "Hi", + }; + }, + }); + + expect(result).toStrictEqual({ + name: "fallback", + test: false, + }); + }); + it("migrate function should receive value", () => { + const json = `{ + "test":"test" + }`; + + const result = parseWithSchema(json, schema, { + migrate: (value) => { + expect(value).toEqual({ test: "test" }); + return { + name: "valid", + }; + }, + }); + + expect(result).toStrictEqual({ + name: "valid", + }); + }); }); }); diff --git a/packages/util/src/json.ts b/packages/util/src/json.ts index 811fcfdbd..97e794bb0 100644 --- a/packages/util/src/json.ts +++ b/packages/util/src/json.ts @@ -1,32 +1,75 @@ import { z, ZodIssue } from "zod"; -import { isZodError } from "./zod"; +import { tryCatchSync } from "./trycatch"; /** * Parse a JSON string into an object and validate it against a schema * @param json JSON string * @param schema Zod schema to validate the JSON against + * @param fallbackAndMigrate Optional object containing optional fallback value and optional migration function + * @throws Error if JSON is invalid and no fallback is provided + * @throws Error if JSON does not match schema and no migration function is provided * @returns The parsed JSON object */ export function parseWithSchema( json: string, - schema: T -): z.infer { - try { - const jsonParsed = JSON.parse(json) as unknown; - // hits is fine to ignore - // eslint-disable-next-line @typescript-eslint/no-unsafe-return - return schema.parse(jsonParsed) as z.infer; - } catch (error) { - if (isZodError(error)) { - throw new Error(error.issues.map(prettyErrorMessage).join("\n")); - } else { - throw error; - } + schema: T, + fallbackAndMigrate?: { + fallback?: z.infer; + migrate?: ( + value: Record | unknown[], + zodIssues?: ZodIssue[] + ) => z.infer; } +): z.infer { + const { fallback, migrate } = fallbackAndMigrate ?? {}; + + const { data: jsonParsed, error } = tryCatchSync( + () => JSON.parse(json) as Record + ); + + if (error) { + if (fallback === undefined) { + throw new Error(`Invalid JSON: ` + error.message); + } + // todo fix me + // eslint-disable-next-line @typescript-eslint/no-unsafe-return + return fallback as z.infer; + } + + const safeParse = schema.safeParse(jsonParsed); + if (safeParse.success) { + return safeParse.data as T; + } + + if (migrate === undefined) { + throw new Error( + `JSON does not match schema: ${safeParse.error.issues + .map(prettyErrorMessage) + .join(", ")}` + ); + } + + const migrated = migrate(jsonParsed, safeParse.error.issues); + const safeParseMigrated = schema.safeParse(migrated); + + if (!safeParseMigrated.success) { + if (fallback === undefined) { + throw new Error( + `Migrated value does not match schema: ${safeParseMigrated.error.issues + .map(prettyErrorMessage) + .join(", ")}` + ); + } + // todo fix me + // eslint-disable-next-line @typescript-eslint/no-unsafe-return + return fallback as z.infer; + } + + return safeParseMigrated.data as T; } function prettyErrorMessage(issue: ZodIssue | undefined): string { if (issue === undefined) return ""; const path = issue.path.length > 0 ? `"${issue.path.join(".")}" ` : ""; - return `${path}${issue.message}`; + return `${path}${issue.message.toLowerCase()}`; }