(fix) Config system and Implementer Tools both crash when 'null' pass…

…ed for Array value (#1101)
openmrs · Jul 31, 2024 · 560741f · 560741f
1 parent f963a9d
commit 560741f
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 5 deletions.
diff --git a/...implementer-tools-app/src/configuration/interactive-editor/value-editors/array-editor.tsx b/...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 (
     <Tile className={styles.arrayEditor}>
       <StructuredListWrapper>
         <StructuredListBody>
-          {valueArray.map((value, i) => (
+          {currentValueArray.map((value, i) => (
             <StructuredListRow key={`${arrayKey}-${i}`}>
               <StructuredListCell>
                 <ValueEditorField
@@ -38,7 +39,7 @@ export function ArrayEditor({ element, valueArray, setValue }: ArrayEditorProps)
                   valueType={(element._elements?._type as Type) ?? Type.Object}
                   value={value}
                   onChange={(newValue) => {
-                    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
@@ -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
@@ -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<string, Function> = {
@@ -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<Function> | undefined, value: any) {
+  let returnValue = true;
   if (validators) {
     try {
       for (let validator of validators) {
@@ -689,12 +703,14 @@ function runValidators(keyPath: string, validators: Array<Function> | 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.