From 560741f45482a7b3a4b6eec3ca5a04525e109893 Mon Sep 17 00:00:00 2001 From: Brandon Istenes Date: Wed, 31 Jul 2024 09:36:25 -0400 Subject: [PATCH] (fix) Config system and Implementer Tools both crash when 'null' passed for Array value (#1101) --- .../value-editors/array-editor.tsx | 9 ++--- .../src/module-config/module-config.test.ts | 34 +++++++++++++++++++ .../src/module-config/module-config.ts | 18 +++++++++- 3 files changed, 56 insertions(+), 5 deletions(-) diff --git a/packages/apps/esm-implementer-tools-app/src/configuration/interactive-editor/value-editors/array-editor.tsx b/packages/apps/esm-implementer-tools-app/src/configuration/interactive-editor/value-editors/array-editor.tsx index 54b4ac8e1..fc64a7581 100644 --- a/packages/apps/esm-implementer-tools-app/src/configuration/interactive-editor/value-editors/array-editor.tsx +++ b/packages/apps/esm-implementer-tools-app/src/configuration/interactive-editor/value-editors/array-editor.tsx @@ -22,11 +22,12 @@ interface ArrayEditorProps { export function ArrayEditor({ element, valueArray, setValue }: ArrayEditorProps) { const arrayKey = useMemo(() => uniqueId('array-editor-'), []); + const currentValueArray = valueArray ?? []; return ( - {valueArray.map((value, i) => ( + {currentValueArray.map((value, i) => ( { - const newValueArray = cloneDeep(valueArray); + const newValueArray = cloneDeep(currentValueArray); newValueArray[i] = newValue; setValue(newValueArray); }} @@ -52,7 +53,7 @@ export function ArrayEditor({ element, valueArray, setValue }: ArrayEditorProps) iconDescription="Remove" hasIconOnly onClick={() => { - const newValueArray = cloneDeep(valueArray); + const newValueArray = cloneDeep(currentValueArray); newValueArray.splice(i, 1); setValue(newValueArray); }} @@ -68,7 +69,7 @@ export function ArrayEditor({ element, valueArray, setValue }: ArrayEditorProps) iconDescription="Add" hasIconOnly onClick={() => { - const newValueArray = cloneDeep(valueArray); + const newValueArray = cloneDeep(currentValueArray); const newValue = (element._elements?._type ?? Type.Object) == Type.Object ? {} : null; newValueArray.push(newValue); setValue(newValueArray); diff --git a/packages/framework/esm-config/src/module-config/module-config.test.ts b/packages/framework/esm-config/src/module-config/module-config.test.ts index d850b9b96..f132bcb37 100644 --- a/packages/framework/esm-config/src/module-config/module-config.test.ts +++ b/packages/framework/esm-config/src/module-config/module-config.test.ts @@ -406,6 +406,23 @@ describe('getConfig', () => { expect(console.error).toHaveBeenCalledWith(expect.stringMatching(/0.*foo-module.*baz.*must be an object/i)); }); + it('does not crash when null is passed as a freeform object config value', async () => { + Config.defineConfigSchema('object-null-module', { + objnu: { + _type: Type.Object, + _default: {}, + }, + }); + const testConfig = { + 'object-null-module': { + objnu: null, + }, + }; + Config.provide(testConfig); + await Config.getConfig('object-null-module'); + expect(console.error).toHaveBeenCalledWith(expect.stringMatching(/.*object-null-module.*obj.*must be an object/i)); + }); + it('supports freeform object elements validations', async () => { Config.defineConfigSchema('foo-module', { foo: { @@ -668,6 +685,23 @@ describe('getConfig', () => { expect(result.bar[0].a.b).toBe(2); }); + it('does not crash when null is passed as an array config value', async () => { + Config.defineConfigSchema('array-null-module', { + arnu: { + _type: Type.Array, + _default: [1, 2, 3], + }, + }); + const testConfig = { + 'array-null-module': { + arnu: null, + }, + }; + Config.provide(testConfig); + await Config.getConfig('array-null-module'); + expect(console.error).toHaveBeenCalledWith(expect.stringMatching(/.*array-null-module.*arnu.*must be an array/i)); + }); + it('fills array element object elements with defaults', async () => { Config.defineConfigSchema('array-def', { foo: { diff --git a/packages/framework/esm-config/src/module-config/module-config.ts b/packages/framework/esm-config/src/module-config/module-config.ts index 805e37c9d..fd91aa045 100644 --- a/packages/framework/esm-config/src/module-config/module-config.ts +++ b/packages/framework/esm-config/src/module-config/module-config.ts @@ -617,6 +617,10 @@ function validateFreeformObjectStructure(freeformObjectSchema: ConfigSchema, con } function validateArrayStructure(arraySchema: ConfigSchema, value: ConfigObject, keyPath: string) { + const validatedAsArray = checkType(keyPath, Type.Array, value); + if (!validatedAsArray) { + return; + } // if there is an array element object schema, verify that elements match it if (hasObjectSchema(arraySchema._elements)) { for (let i = 0; i < value.length; i++) { @@ -660,6 +664,10 @@ function runAllValidatorsInConfigTree(schema: ConfigSchema, config: ConfigObject } } +/** + * Run type validation for the value, logging any errors. + * @returns true if validation passes, false otherwise + */ function checkType(keyPath: string, _type: Type | undefined, value: any) { if (_type) { const validator: Record = { @@ -673,11 +681,17 @@ function checkType(keyPath: string, _type: Type | undefined, value: any) { PersonAttributeTypeUuid: isUuid, PatientIdentifierTypeUuid: isUuid, }; - runValidators(keyPath, [validator[_type]], value); + return runValidators(keyPath, [validator[_type]], value); } + return true; } +/** + * Runs validators, logging errors. + * @returns true if all pass, false otherwise. + */ function runValidators(keyPath: string, validators: Array | undefined, value: any) { + let returnValue = true; if (validators) { try { for (let validator of validators) { @@ -689,12 +703,14 @@ function runValidators(keyPath: string, validators: Array | undefined, ? `Invalid configuration for ${keyPath}: ${validatorResult}` : `Invalid configuration value ${value} for ${keyPath}: ${validatorResult}`; logError(keyPath, message); + returnValue = false; } } } catch (e) { console.error(`Skipping invalid validator at "${keyPath}". Encountered error\n\t${e}`); } } + return returnValue; } // Recursively fill in the config with values from the schema.