implement suggested changes as per review

Koenkk · Jun 14, 2024 · d65a611 · d65a611
1 parent f7f5bc4
commit d65a611
Show file tree

Hide file tree

Showing 3 changed files with 85 additions and 7 deletions.
diff --git a/lib/util/settings.ts b/lib/util/settings.ts
@@ -32,7 +32,7 @@ export type LogLevel = typeof LOG_LEVELS[number];
 
 // DEPRECATED ZIGBEE2MQTT_CONFIG: https://github.com/Koenkk/zigbee2mqtt/issues/4697
 const file = process.env.ZIGBEE2MQTT_CONFIG ?? data.joinPath('configuration.yaml');
-const nullPropertiesIgnoreList = ['homeassistant'];
+const NULLABLE_SETTINGS = ['homeassistant'];
 const ajvSetting = new Ajv({allErrors: true}).addKeyword('requiresRestart').compile(schemaJson);
 const ajvRestartRequired = new Ajv({allErrors: true})
     .addKeyword({keyword: 'requiresRestart', validate: (s: unknown) => !s}).compile(schemaJson);
@@ -485,7 +485,7 @@ export function apply(settings: Record<string, unknown>): boolean {
     getInternalSettings(); // Ensure _settings is initialized.
     /* eslint-disable-line */ // @ts-ignore
     const newSettings = objectAssignDeep.noMutate(_settings, settings);
-    utils.removeNullPropertiesFromObject(newSettings, nullPropertiesIgnoreList);
+    utils.removeNullPropertiesFromObject(newSettings, NULLABLE_SETTINGS);
     ajvSetting(newSettings);
     const errors = ajvSetting.errors && ajvSetting.errors.filter((e) => e.keyword !== 'required');
     if (errors?.length) {
@@ -696,11 +696,11 @@ export function changeEntityOptions(IDorName: string, newOptions: KeyValue): boo
     let validator: ValidateFunction;
     if (getDevice(IDorName)) {
         objectAssignDeep(settings.devices[getDevice(IDorName).ID], newOptions);
-        utils.removeNullPropertiesFromObject(settings.devices[getDevice(IDorName).ID], nullPropertiesIgnoreList);
+        utils.removeNullPropertiesFromObject(settings.devices[getDevice(IDorName).ID], NULLABLE_SETTINGS);
         validator = ajvRestartRequiredDeviceOptions;
     } else if (getGroup(IDorName)) {
         objectAssignDeep(settings.groups[getGroup(IDorName).ID], newOptions);
-        utils.removeNullPropertiesFromObject(settings.groups[getGroup(IDorName).ID], nullPropertiesIgnoreList );
+        utils.removeNullPropertiesFromObject(settings.groups[getGroup(IDorName).ID], NULLABLE_SETTINGS );
         validator = ajvRestartRequiredGroupOptions;
     } else {
         throw new Error(`Device or group '${IDorName}' does not exist`);

diff --git a/lib/util/utils.ts b/lib/util/utils.ts
@@ -162,14 +162,20 @@ export function* loadExternalConverter(moduleName: string): Generator<ExternalDe
     }
 }
 
-function removeNullPropertiesFromObject(obj: KeyValue, ignorePaths: string[] = [] ): void {
+/**
+ * Delete all keys from passed object that have null/undefined values.
+ *
+ * @param {KeyValue} obj Object to process (in-place)
+ * @param {string[]} [ignoreKeys] Recursively ignore these keys in the object (keep null/undefined values).
+ */
+function removeNullPropertiesFromObject(obj: KeyValue, ignoreKeys: string[] = [] ): void {
     for (const key of Object.keys(obj)) {
-        if (ignorePaths.includes(key)) continue;
+        if (ignoreKeys.includes(key)) continue;
         const value = obj[key];
         if (value == null) {
             delete obj[key];
         } else if (typeof value === 'object') {
-            removeNullPropertiesFromObject(value, ignorePaths);
+            removeNullPropertiesFromObject(value, ignoreKeys);
         }
     }
 }

diff --git a/test/utils.test.js b/test/utils.test.js
@@ -36,4 +36,76 @@ describe('Utils', () => {
         expect(utils.formatDate(date, 'ISO_8601_local').endsWith('+01:00')).toBeTruthy();
         Date.prototype.getTimezoneOffset = getTimezoneOffset;
     })
+    it('Removes null properties from object', () => {
+        const obj1 = {
+            ab: 0,
+            cd: false,
+            ef: null,
+            gh: '',
+            homeassistant: {
+                xyz: 'mock',
+                abcd: null,
+            },
+            nested: {
+                homeassistant: {
+                    abcd: true,
+                    xyz: null,
+                },
+                abc: {},
+                def: null,
+            },
+        };
+
+        utils.removeNullPropertiesFromObject(obj1);
+        expect(obj1).toStrictEqual({
+            ab: 0,
+            cd: false,
+            gh: '',
+            homeassistant: {
+                xyz: 'mock',
+            },
+            nested: {
+                homeassistant: {
+                    abcd: true,
+                },
+                abc: {},
+            },
+        });
+
+        const obj2 = {
+            ab: 0,
+            cd: false,
+            ef: null,
+            gh: '',
+            homeassistant: {
+                xyz: 'mock',
+                abcd: null,
+            },
+            nested: {
+                homeassistant: {
+                    abcd: true,
+                    xyz: null,
+                },
+                abc: {},
+                def: null,
+            },
+        };
+        utils.removeNullPropertiesFromObject(obj2, ['homeassistant']);
+        expect(obj2).toStrictEqual({
+            ab: 0,
+            cd: false,
+            gh: '',
+            homeassistant: {
+                xyz: 'mock',
+                abcd: null,
+            },
+            nested: {
+                homeassistant: {
+                    abcd: true,
+                    xyz: null,
+                },
+                abc: {},
+            },
+        });
+    });
 });