From 7fd5f0cb0c395874a4d92c1d16dbc0b6b864ae89 Mon Sep 17 00:00:00 2001 From: Nerivec <62446222+Nerivec@users.noreply.github.com> Date: Wed, 26 Feb 2025 22:07:37 +0100 Subject: [PATCH 1/7] feat: Use dynamic import for external JS --- lib/extension/externalConverters.ts | 20 +- lib/extension/externalExtensions.ts | 19 +- lib/extension/externalJS.ts | 63 +-- .../mock-external-converter-multiple.js | 0 .../{ => cjs}/mock-external-converter.js | 4 +- .../mjs/mock-external-converter-multiple.mjs | 22 + .../mjs/mock-external-converter.mjs | 12 + .../{ => cjs}/example2Extension.js | 0 .../{ => cjs}/exampleExtension.js | 0 .../mjs/example2Extension.mjs | 16 + .../mjs/exampleExtension.mjs | 16 + test/extensions/externalConverters.test.ts | 476 ++++++++++++++---- test/extensions/externalExtensions.test.ts | 263 +++++++--- test/extensions/networkMap.test.ts | 27 +- 14 files changed, 711 insertions(+), 227 deletions(-) rename test/assets/external_converters/{ => cjs}/mock-external-converter-multiple.js (100%) rename test/assets/external_converters/{ => cjs}/mock-external-converter.js (71%) create mode 100644 test/assets/external_converters/mjs/mock-external-converter-multiple.mjs create mode 100644 test/assets/external_converters/mjs/mock-external-converter.mjs rename test/assets/external_extensions/{ => cjs}/example2Extension.js (100%) rename test/assets/external_extensions/{ => cjs}/exampleExtension.js (100%) create mode 100644 test/assets/external_extensions/mjs/example2Extension.mjs create mode 100644 test/assets/external_extensions/mjs/exampleExtension.mjs diff --git a/lib/extension/externalConverters.ts b/lib/extension/externalConverters.ts index e281349d3e..fe92f7844a 100644 --- a/lib/extension/externalConverters.ts +++ b/lib/extension/externalConverters.ts @@ -1,13 +1,13 @@ -import type * as zhc from 'zigbee-herdsman-converters'; +import type {ExternalDefinitionWithExtend} from 'zigbee-herdsman-converters'; import {addExternalDefinition, removeExternalDefinitions} from 'zigbee-herdsman-converters'; import logger from '../util/logger'; import ExternalJSExtension from './externalJS'; -type ModuleExports = zhc.ExternalDefinitionWithExtend | zhc.ExternalDefinitionWithExtend[]; +type TModule = ExternalDefinitionWithExtend | ExternalDefinitionWithExtend[]; -export default class ExternalConverters extends ExternalJSExtension { +export default class ExternalConverters extends ExternalJSExtension { constructor( zigbee: Zigbee, mqtt: MQTT, @@ -33,27 +33,29 @@ export default class ExternalConverters extends ExternalJSExtension { + protected async removeJS(name: string, mod: TModule): Promise { removeExternalDefinitions(name); await this.zigbee.resolveDevicesDefinitions(true); } - protected async loadJS(name: string, module: ModuleExports): Promise { + protected async loadJS(name: string, mod: TModule, newName?: string): Promise { try { removeExternalDefinitions(name); - const definitions = Array.isArray(module) ? module : [module]; + const definitions = Array.isArray(mod) ? mod : [mod]; + for (const definition of definitions) { - definition.externalConverterName = name; + definition.externalConverterName = newName ?? name; addExternalDefinition(definition); - logger.info(`Loaded external converter '${name}'.`); + logger.info(`Loaded external converter '${newName ?? name}'.`); } await this.zigbee.resolveDevicesDefinitions(true); } catch (error) { - logger.error(`Failed to load external converter '${name}'`); + /* v8 ignore next */ + logger.error(`Failed to load external converter '${newName ?? name}'`); logger.error(`Check the code for syntax error and make sure it is up to date with the current Zigbee2MQTT version.`); logger.error( `External converters are not meant for long term usage, but for local testing after which a pull request should be created to add out-of-the-box support for the device`, diff --git a/lib/extension/externalExtensions.ts b/lib/extension/externalExtensions.ts index 3537617b34..e5baedba59 100644 --- a/lib/extension/externalExtensions.ts +++ b/lib/extension/externalExtensions.ts @@ -4,9 +4,9 @@ import logger from '../util/logger'; import * as settings from '../util/settings'; import ExternalJSExtension from './externalJS'; -type ModuleExports = typeof Extension; +type TModule = new (...args: ConstructorParameters) => Extension; -export default class ExternalExtensions extends ExternalJSExtension { +export default class ExternalExtensions extends ExternalJSExtension { constructor( zigbee: Zigbee, mqtt: MQTT, @@ -31,16 +31,15 @@ export default class ExternalExtensions extends ExternalJSExtension { - await this.enableDisableExtension(false, module.name); + protected async removeJS(name: string, mod: TModule): Promise { + await this.enableDisableExtension(false, mod.name); } - protected async loadJS(name: string, module: ModuleExports): Promise { + protected async loadJS(name: string, mod: TModule, newName?: string): Promise { // stop if already started - await this.enableDisableExtension(false, module.name); + await this.enableDisableExtension(false, mod.name); await this.addExtension( - // @ts-expect-error `module` is the interface, not the actual passed class - new module( + new mod( this.zigbee, this.mqtt, this.state, @@ -49,11 +48,13 @@ export default class ExternalExtensions extends ExternalJSExtension extends Extension { } for (const fileName of fs.readdirSync(this.basePath)) { - if (fileName.endsWith('.js')) { + if (fileName.endsWith('.js') || fileName.endsWith('.cjs') || fileName.endsWith('.mjs')) { yield {name: fileName, code: this.getFileCode(fileName)}; } } @@ -100,9 +99,9 @@ export default abstract class ExternalJSExtension extends Extension { } } - protected abstract removeJS(name: string, module: M): Promise; + protected abstract removeJS(name: string, mod: M): Promise; - protected abstract loadJS(name: string, module: M): Promise; + protected abstract loadJS(name: string, mod: M, newName?: string): Promise; @bind private async remove( message: Zigbee2MQTTAPI['bridge/request/converter/remove'] | Zigbee2MQTTAPI['bridge/request/extension/remove'], @@ -115,8 +114,9 @@ export default abstract class ExternalJSExtension extends Extension { const toBeRemoved = this.getFilePath(name); if (fs.existsSync(toBeRemoved)) { - await this.removeJS(name, this.loadModuleFromText(this.getFileCode(name), name)); + const mod = await import(toBeRemoved); + await this.removeJS(name, mod.default); fs.rmSync(toBeRemoved, {force: true}); logger.info(`${name} (${toBeRemoved}) removed.`); await this.publishExternalJS(); @@ -135,25 +135,47 @@ export default abstract class ExternalJSExtension extends Extension { } const {name, code} = message; + const filePath = this.getFilePath(name, true); + let newFilePath = filePath; + let newName = name; + + if (fs.existsSync(filePath)) { + // if file already exist, version it to bypass node module caching + const versionMatch = name.match(/\.(\d+)\.(c|m)?js$/); + + if (versionMatch) { + const version = parseInt(versionMatch[1], 10); + newName = name.replace(`.${version}.`, `.${version + 1}.`); + } else { + const ext = path.extname(name); + newName = name.replace(ext, `.1${ext}`); + } + + newFilePath = this.getFilePath(newName, true); + } try { - await this.loadJS(name, this.loadModuleFromText(code, name)); + fs.writeFileSync(newFilePath, code, 'utf8'); - const filePath = this.getFilePath(name, true); + const mod = await import(newFilePath); - fs.writeFileSync(filePath, code, 'utf8'); - logger.info(`${name} loaded. Contents written to '${filePath}'.`); + await this.loadJS(name, mod.default, newName); + logger.info(`${newName} loaded. Contents written to '${newFilePath}'.`); await this.publishExternalJS(); return utils.getResponse(message, {}); } catch (error) { - return utils.getResponse(message, {}, `${name} contains invalid code: ${(error as Error).message}`); + fs.rmSync(newFilePath, {force: true}); + + return utils.getResponse(message, {}, `${newName} contains invalid code: ${(error as Error).message}`); } } private async loadFiles(): Promise { for (const extension of this.getFiles()) { - await this.loadJS(extension.name, this.loadModuleFromText(extension.code, extension.name)); + const mod = await import(path.join(this.basePath, extension.name)); + + await this.loadJS(extension.name, mod.default); } } @@ -169,23 +191,4 @@ export default abstract class ExternalJSExtension extends Extension { true, ); } - - private loadModuleFromText(moduleCode: string, name: string): M { - const moduleFakePath = path.join(__dirname, '..', '..', 'data', 'extension', name); - const sandbox: Context = { - require: require, - module: {}, - console, - setTimeout, - clearTimeout, - setInterval, - clearInterval, - setImmediate, - clearImmediate, - }; - - runInNewContext(moduleCode, sandbox, moduleFakePath); - - return sandbox.module.exports; - } } diff --git a/test/assets/external_converters/mock-external-converter-multiple.js b/test/assets/external_converters/cjs/mock-external-converter-multiple.js similarity index 100% rename from test/assets/external_converters/mock-external-converter-multiple.js rename to test/assets/external_converters/cjs/mock-external-converter-multiple.js diff --git a/test/assets/external_converters/mock-external-converter.js b/test/assets/external_converters/cjs/mock-external-converter.js similarity index 71% rename from test/assets/external_converters/mock-external-converter.js rename to test/assets/external_converters/cjs/mock-external-converter.js index c2f0437721..d90b52c0b9 100644 --- a/test/assets/external_converters/mock-external-converter.js +++ b/test/assets/external_converters/cjs/mock-external-converter.js @@ -1,9 +1,11 @@ +const {posix} = require('node:path'); + const mockDevice = { mock: true, zigbeeModel: ['external_converter_device'], vendor: 'external', model: 'external_converter_device', - description: 'external', + description: posix.join('external', 'converter'), fromZigbee: [], toZigbee: [], exposes: [], diff --git a/test/assets/external_converters/mjs/mock-external-converter-multiple.mjs b/test/assets/external_converters/mjs/mock-external-converter-multiple.mjs new file mode 100644 index 0000000000..4e71c681fa --- /dev/null +++ b/test/assets/external_converters/mjs/mock-external-converter-multiple.mjs @@ -0,0 +1,22 @@ +export default [ + { + mock: 1, + model: 'external_converters_device_1', + zigbeeModel: ['external_converter_device_1'], + vendor: 'external_1', + description: 'external_1', + fromZigbee: [], + toZigbee: [], + exposes: [], + }, + { + mock: 2, + model: 'external_converters_device_2', + zigbeeModel: ['external_converter_device_2'], + vendor: 'external_2', + description: 'external_2', + fromZigbee: [], + toZigbee: [], + exposes: [], + }, +]; diff --git a/test/assets/external_converters/mjs/mock-external-converter.mjs b/test/assets/external_converters/mjs/mock-external-converter.mjs new file mode 100644 index 0000000000..099e9c62a0 --- /dev/null +++ b/test/assets/external_converters/mjs/mock-external-converter.mjs @@ -0,0 +1,12 @@ +import {posix} from 'node:path'; + +export default { + mock: true, + zigbeeModel: ['external_converter_device'], + vendor: 'external', + model: 'external_converter_device', + description: posix.join('external', 'converter'), + fromZigbee: [], + toZigbee: [], + exposes: [], +}; diff --git a/test/assets/external_extensions/example2Extension.js b/test/assets/external_extensions/cjs/example2Extension.js similarity index 100% rename from test/assets/external_extensions/example2Extension.js rename to test/assets/external_extensions/cjs/example2Extension.js diff --git a/test/assets/external_extensions/exampleExtension.js b/test/assets/external_extensions/cjs/exampleExtension.js similarity index 100% rename from test/assets/external_extensions/exampleExtension.js rename to test/assets/external_extensions/cjs/exampleExtension.js diff --git a/test/assets/external_extensions/mjs/example2Extension.mjs b/test/assets/external_extensions/mjs/example2Extension.mjs new file mode 100644 index 0000000000..4ac629db47 --- /dev/null +++ b/test/assets/external_extensions/mjs/example2Extension.mjs @@ -0,0 +1,16 @@ +class Example2 { + constructor(zigbee, mqtt, state, publishEntityState, eventBus) { + this.mqtt = mqtt; + this.mqtt.publish('example2/extension', 'call2 from constructor'); + } + + start() { + this.mqtt.publish('example2/extension', 'call2 from start'); + } + + stop() { + this.mqtt.publish('example/extension', 'call2 from stop'); + } +} + +export default Example2; diff --git a/test/assets/external_extensions/mjs/exampleExtension.mjs b/test/assets/external_extensions/mjs/exampleExtension.mjs new file mode 100644 index 0000000000..40e373f78f --- /dev/null +++ b/test/assets/external_extensions/mjs/exampleExtension.mjs @@ -0,0 +1,16 @@ +class Example { + constructor(zigbee, mqtt, state, publishEntityState, eventBus) { + this.mqtt = mqtt; + this.mqtt.publish('example/extension', 'call from constructor'); + } + + start() { + this.mqtt.publish('example/extension', 'call from start'); + } + + stop() { + this.mqtt.publish('example/extension', 'call from stop'); + } +} + +export default Example; diff --git a/test/extensions/externalConverters.test.ts b/test/extensions/externalConverters.test.ts index e4c6790fd9..d501c4bd48 100644 --- a/test/extensions/externalConverters.test.ts +++ b/test/extensions/externalConverters.test.ts @@ -1,9 +1,10 @@ import * as data from '../mocks/data'; import {mockLogger} from '../mocks/logger'; -import {mockMQTTEndAsync, events as mockMQTTEvents, mockMQTTPublishAsync} from '../mocks/mqtt'; +import {mockMQTTEndAsync, mockMQTTPublishAsync} from '../mocks/mqtt'; import {flushPromises} from '../mocks/utils'; import {devices, mockController as mockZHController, returnDevices} from '../mocks/zigbeeHerdsman'; +import type ExternalConverters from '../../lib/extension/externalConverters'; import type Device from '../../lib/model/device'; import fs from 'node:fs'; @@ -48,12 +49,17 @@ describe('Extension: ExternalConverters', () => { zhcRemoveExternalDefinitionsSpy, ]; - const useAssets = (): void => { - fs.cpSync(path.join(__dirname, '..', 'assets', BASE_DIR), mockBasePath, {recursive: true}); + const getExtension = (): ExternalConverters => { + // @ts-expect-error private + return controller.extensions.find((e) => e.constructor.name === 'ExternalConverters'); + }; + + const useAssets = (mtype: 'cjs' | 'mjs'): void => { + fs.cpSync(path.join(__dirname, '..', 'assets', BASE_DIR, mtype), mockBasePath, {recursive: true}); }; - const getFileCode = (fileName: string): string => { - return fs.readFileSync(path.join(__dirname, '..', 'assets', BASE_DIR, fileName), 'utf8'); + const getFileCode = (mtype: 'cjs' | 'mjs', fileName: string): string => { + return fs.readFileSync(path.join(__dirname, '..', 'assets', BASE_DIR, mtype, fileName), 'utf8'); }; const getZ2MDevice = (zhDevice: unknown): Device => { @@ -80,6 +86,8 @@ describe('Extension: ExternalConverters', () => { beforeEach(async () => { zhc.removeExternalDefinitions(); // remove all external converters + // @ts-expect-error private - clear cached + await controller.zigbee.resolveDevicesDefinitions(true); mocksClear.forEach((m) => m.mockClear()); data.writeDefaultConfiguration(); data.writeDefaultState(); @@ -107,14 +115,14 @@ describe('Extension: ExternalConverters', () => { expect(mockMQTTPublishAsync).toHaveBeenCalledWith('zigbee2mqtt/bridge/converters', stringify([]), {retain: true, qos: 0}); }); - it('loads converters', async () => { - useAssets(); + it('CJS: loads converters', async () => { + useAssets('cjs'); await controller.start(); await flushPromises(); expect(getZ2MDevice(devices.external_converter_device).definition).toMatchObject({ - description: 'external', + description: 'external/converter', model: 'external_converter_device', vendor: 'external', zigbeeModel: ['external_converter_device'], @@ -122,8 +130,8 @@ describe('Extension: ExternalConverters', () => { expect(mockMQTTPublishAsync).toHaveBeenCalledWith( 'zigbee2mqtt/bridge/converters', stringify([ - {name: 'mock-external-converter-multiple.js', code: getFileCode('mock-external-converter-multiple.js')}, - {name: 'mock-external-converter.js', code: getFileCode('mock-external-converter.js')}, + {name: 'mock-external-converter-multiple.js', code: getFileCode('cjs', 'mock-external-converter-multiple.js')}, + {name: 'mock-external-converter.js', code: getFileCode('cjs', 'mock-external-converter.js')}, ]), {retain: true, qos: 0}, ); @@ -157,7 +165,7 @@ describe('Extension: ExternalConverters', () => { zigbeeModel: ['external_converter_device'], vendor: 'external', model: 'external_converter_device', - description: 'external', + description: 'external/converter', }), ); @@ -169,7 +177,7 @@ describe('Extension: ExternalConverters', () => { model_id: 'external_converter_device', supported: true, definition: expect.objectContaining({ - description: 'external', + description: 'external/converter', model: 'external_converter_device', }), }), @@ -177,13 +185,156 @@ describe('Extension: ExternalConverters', () => { ); }); - it('saves and removes from MQTT', async () => { - const converterName = 'foo.js'; - const converterCode = getFileCode('mock-external-converter.js'); - const converterFilePath = path.join(mockBasePath, converterName); + it('MJS: loads converters', async () => { + useAssets('mjs'); + + await controller.start(); + await flushPromises(); + + expect(getZ2MDevice(devices.external_converter_device).definition).toMatchObject({ + description: 'external/converter', + model: 'external_converter_device', + vendor: 'external', + zigbeeModel: ['external_converter_device'], + }); + expect(mockMQTTPublishAsync).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/converters', + stringify([ + {name: 'mock-external-converter-multiple.mjs', code: getFileCode('mjs', 'mock-external-converter-multiple.mjs')}, + {name: 'mock-external-converter.mjs', code: getFileCode('mjs', 'mock-external-converter.mjs')}, + ]), + {retain: true, qos: 0}, + ); + expect(zhcRemoveExternalDefinitionsSpy).toHaveBeenCalledTimes(2); + expect(zhcRemoveExternalDefinitionsSpy).toHaveBeenNthCalledWith(1, 'mock-external-converter-multiple.mjs'); + expect(zhcRemoveExternalDefinitionsSpy).toHaveBeenNthCalledWith(2, 'mock-external-converter.mjs'); + expect(zhcAddExternalDefinitionSpy).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + mock: 1, + model: 'external_converters_device_1', + zigbeeModel: ['external_converter_device_1'], + vendor: 'external_1', + description: 'external_1', + }), + ); + expect(zhcAddExternalDefinitionSpy).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + mock: 2, + model: 'external_converters_device_2', + zigbeeModel: ['external_converter_device_2'], + vendor: 'external_2', + description: 'external_2', + }), + ); + expect(zhcAddExternalDefinitionSpy).toHaveBeenNthCalledWith( + 3, + expect.objectContaining({ + mock: true, + zigbeeModel: ['external_converter_device'], + vendor: 'external', + model: 'external_converter_device', + description: 'external/converter', + }), + ); + + const bridgeDevices = mockMQTTPublishAsync.mock.calls.filter((c) => c[0] === 'zigbee2mqtt/bridge/devices'); + expect(bridgeDevices.length).toBe(1); + expect(JSON.parse(bridgeDevices[0][1])).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + model_id: 'external_converter_device', + supported: true, + definition: expect.objectContaining({ + description: 'external/converter', + model: 'external_converter_device', + }), + }), + ]), + ); + }); + + it('updates after edit from MQTT', async () => { + const converterName = 'mock-external-converter.js'; + let converterCode = getFileCode('cjs', 'mock-external-converter.js'); + useAssets('cjs'); await controller.start(); await flushPromises(); + + expect(getZ2MDevice(devices.external_converter_device).definition).toMatchObject({ + description: 'external/converter', + model: 'external_converter_device', + vendor: 'external', + zigbeeModel: ['external_converter_device'], + }); + expect(mockMQTTPublishAsync).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/converters', + stringify([ + {name: 'mock-external-converter-multiple.js', code: getFileCode('cjs', 'mock-external-converter-multiple.js')}, + {name: converterName, code: getFileCode('cjs', converterName)}, + ]), + {retain: true, qos: 0}, + ); + + converterCode = converterCode.replace("posix.join('external', 'converter')", "posix.join('external', 'converter', 'edited')"); + + await getExtension().onMQTTMessage({ + topic: 'zigbee2mqtt/bridge/request/converter/save', + message: {name: converterName, code: converterCode}, + }); + + expect(getZ2MDevice(devices.external_converter_device).definition).toMatchObject({ + description: 'external/converter/edited', + model: 'external_converter_device', + vendor: 'external', + zigbeeModel: ['external_converter_device'], + }); + expect(zhcAddExternalDefinitionSpy).toHaveBeenLastCalledWith( + expect.objectContaining({ + mock: true, + zigbeeModel: ['external_converter_device'], + vendor: 'external', + model: 'external_converter_device', + description: 'external/converter/edited', + externalConverterName: 'mock-external-converter.1.js', + }), + ); + + converterCode = converterCode.replace("posix.join('external', 'converter', 'edited')", "posix.join('external', 'converter')"); + + await getExtension().onMQTTMessage({ + topic: 'zigbee2mqtt/bridge/request/converter/save', + message: {name: 'mock-external-converter.1.js', code: converterCode}, + }); + + expect(getZ2MDevice(devices.external_converter_device).definition).toMatchObject({ + description: 'external/converter', + model: 'external_converter_device', + vendor: 'external', + zigbeeModel: ['external_converter_device'], + }); + expect(zhcAddExternalDefinitionSpy).toHaveBeenLastCalledWith( + expect.objectContaining({ + mock: true, + zigbeeModel: ['external_converter_device'], + vendor: 'external', + model: 'external_converter_device', + description: 'external/converter', + externalConverterName: 'mock-external-converter.2.js', + }), + ); + }); + }); + + describe('from MQTT', () => { + it('CJS: saves and removes', async () => { + const converterName = 'foo.js'; + const converterCode = getFileCode('cjs', 'mock-external-converter.js'); + const converterFilePath = path.join(mockBasePath, converterName); + + await resetExtension(); mocksClear.forEach((m) => m.mockClear()); expect(getZ2MDevice(devices.external_converter_device).definition).toMatchObject({ @@ -194,11 +345,15 @@ describe('Extension: ExternalConverters', () => { }); //-- SAVE - mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/save', stringify({name: converterName, code: converterCode})); - await flushPromises(); + // await mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/save', stringify({name: converterName, code: converterCode})); + // await flushPromises(); + await getExtension().onMQTTMessage({ + topic: 'zigbee2mqtt/bridge/request/converter/save', + message: {name: converterName, code: converterCode}, + }); expect(getZ2MDevice(devices.external_converter_device).definition).toMatchObject({ - description: 'external', + description: 'external/converter', model: 'external_converter_device', vendor: 'external', zigbeeModel: ['external_converter_device'], @@ -213,7 +368,7 @@ describe('Extension: ExternalConverters', () => { zigbeeModel: ['external_converter_device'], vendor: 'external', model: 'external_converter_device', - description: 'external', + description: 'external/converter', }), ); expect(mockMQTTPublishAsync).toHaveBeenCalledWith( @@ -226,8 +381,12 @@ describe('Extension: ExternalConverters', () => { ); //-- REMOVE - mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/remove', stringify({name: converterName})); - await flushPromises(); + // await mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/remove', stringify({name: converterName})); + // await flushPromises(); + await getExtension().onMQTTMessage({ + topic: 'zigbee2mqtt/bridge/request/converter/remove', + message: {name: converterName}, + }); expect(getZ2MDevice(devices.external_converter_device).definition).toMatchObject({ description: 'Automatically generated definition', @@ -240,119 +399,220 @@ describe('Extension: ExternalConverters', () => { expect(zhcRemoveExternalDefinitionsSpy).toHaveBeenNthCalledWith(2, converterName); expect(mockMQTTPublishAsync).toHaveBeenCalledWith('zigbee2mqtt/bridge/converters', stringify([]), {retain: true, qos: 0}); }); - }); - - it('returns error on invalid code', async () => { - const converterName = 'foo.js'; - const converterCode = 'definetly not a correct javascript code'; - const converterFilePath = path.join(mockBasePath, converterName); - await resetExtension(); - mocksClear.forEach((m) => m.mockClear()); + it('MJS: saves and removes', async () => { + const converterName = 'foo.mjs'; + const converterCode = getFileCode('mjs', 'mock-external-converter.mjs'); + const converterFilePath = path.join(mockBasePath, converterName); - mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/save', stringify({name: converterName, code: converterCode})); - await flushPromises(); + await resetExtension(); + mocksClear.forEach((m) => m.mockClear()); - expect(mockMQTTPublishAsync).toHaveBeenCalledWith( - 'zigbee2mqtt/bridge/response/converter/save', - expect.stringContaining(`"error":"foo.js contains invalid code`), - {retain: false, qos: 0}, - ); - expect(writeFileSyncSpy).not.toHaveBeenCalledWith(converterFilePath, converterCode, 'utf8'); - }); + expect(getZ2MDevice(devices.external_converter_device).definition).toMatchObject({ + description: 'Automatically generated definition', + model: 'external_converter_device', + vendor: '', + zigbeeModel: ['external_converter_device'], + }); - it('returns error on invalid removal', async () => { - const converterName = 'invalid.js'; - const converterFilePath = path.join(mockBasePath, converterName); + //-- SAVE + // await mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/save', stringify({name: converterName, code: converterCode})); + // await flushPromises(); + await getExtension().onMQTTMessage({ + topic: 'zigbee2mqtt/bridge/request/converter/save', + message: {name: converterName, code: converterCode}, + }); - await resetExtension(); - mocksClear.forEach((m) => m.mockClear()); + expect(getZ2MDevice(devices.external_converter_device).definition).toMatchObject({ + description: 'external/converter', + model: 'external_converter_device', + vendor: 'external', + zigbeeModel: ['external_converter_device'], + }); + expect(mkdirSyncSpy).toHaveBeenCalledWith(mockBasePath, {recursive: true}); + expect(writeFileSyncSpy).toHaveBeenCalledWith(converterFilePath, converterCode, 'utf8'); + expect(zhcRemoveExternalDefinitionsSpy).toHaveBeenCalledTimes(1); + expect(zhcRemoveExternalDefinitionsSpy).toHaveBeenNthCalledWith(1, converterName); + expect(zhcAddExternalDefinitionSpy).toHaveBeenCalledWith( + expect.objectContaining({ + mock: true, + zigbeeModel: ['external_converter_device'], + vendor: 'external', + model: 'external_converter_device', + description: 'external/converter', + }), + ); + expect(mockMQTTPublishAsync).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/converters', + stringify([{name: converterName, code: converterCode}]), + { + retain: true, + qos: 0, + }, + ); - mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/remove', stringify({name: converterName})); - await flushPromises(); + //-- REMOVE + // await mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/remove', stringify({name: converterName})); + // await flushPromises(); + await getExtension().onMQTTMessage({ + topic: 'zigbee2mqtt/bridge/request/converter/remove', + message: {name: converterName}, + }); - expect(mockMQTTPublishAsync).toHaveBeenCalledWith( - 'zigbee2mqtt/bridge/response/converter/remove', - stringify({data: {}, status: 'error', error: `${converterName} (${converterFilePath}) doesn't exists`}), - {retain: false, qos: 0}, - ); - expect(rmSyncSpy).not.toHaveBeenCalledWith(converterFilePath, {force: true}); - }); + expect(getZ2MDevice(devices.external_converter_device).definition).toMatchObject({ + description: 'Automatically generated definition', + model: 'external_converter_device', + vendor: '', + zigbeeModel: ['external_converter_device'], + }); + expect(rmSyncSpy).toHaveBeenCalledWith(converterFilePath, {force: true}); + expect(zhcRemoveExternalDefinitionsSpy).toHaveBeenCalledTimes(2); + expect(zhcRemoveExternalDefinitionsSpy).toHaveBeenNthCalledWith(2, converterName); + expect(mockMQTTPublishAsync).toHaveBeenCalledWith('zigbee2mqtt/bridge/converters', stringify([]), {retain: true, qos: 0}); + }); - it('returns error on invalid definition', async () => { - const converterName = 'foo.js'; - const converterCode = getFileCode('mock-external-converter.js'); - const converterFilePath = path.join(mockBasePath, converterName); + it('returns error on invalid code', async () => { + const converterName = 'foo1.js'; + const converterCode = 'definetly not a correct javascript code'; + const converterFilePath = path.join(mockBasePath, converterName); - await resetExtension(); - mocksClear.forEach((m) => m.mockClear()); + await resetExtension(); + mocksClear.forEach((m) => m.mockClear()); - const errorMsg = `Invalid definition`; + // await mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/save', stringify({name: converterName, code: converterCode})); + // await flushPromises(); + await getExtension().onMQTTMessage({ + topic: 'zigbee2mqtt/bridge/request/converter/save', + message: {name: converterName, code: converterCode}, + }); - zhcAddExternalDefinitionSpy.mockImplementationOnce(() => { - throw new Error(errorMsg); + expect(mockMQTTPublishAsync).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/response/converter/save', + expect.stringContaining(`"error":"${converterName} contains invalid code`), + {retain: false, qos: 0}, + ); + expect(writeFileSyncSpy).toHaveBeenCalledWith(converterFilePath, converterCode, 'utf8'); + expect(rmSyncSpy).toHaveBeenCalledWith(converterFilePath, {force: true}); }); - mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/save', stringify({name: converterName, code: converterCode})); - await flushPromises(); + it('returns error on invalid removal', async () => { + const converterName = 'foo2.js'; + const converterFilePath = path.join(mockBasePath, converterName); - expect(mockMQTTPublishAsync).toHaveBeenCalledWith('zigbee2mqtt/bridge/response/converter/save', expect.stringContaining(errorMsg), { - retain: false, - qos: 0, + await resetExtension(); + mocksClear.forEach((m) => m.mockClear()); + + // await mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/remove', stringify({name: converterName})); + // await flushPromises(); + await getExtension().onMQTTMessage({ + topic: 'zigbee2mqtt/bridge/request/converter/remove', + message: {name: converterName}, + }); + + expect(mockMQTTPublishAsync).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/response/converter/remove', + stringify({data: {}, status: 'error', error: `${converterName} (${converterFilePath}) doesn't exists`}), + {retain: false, qos: 0}, + ); + expect(rmSyncSpy).not.toHaveBeenCalledWith(converterFilePath, {force: true}); }); - expect(writeFileSyncSpy).not.toHaveBeenCalledWith(converterFilePath, converterCode, 'utf8'); - }); - it('returns error on failed removal', async () => { - const converterName = 'foo.js'; - const converterCode = getFileCode('mock-external-converter.js'); - const converterFilePath = path.join(mockBasePath, converterName); + it('returns error on invalid definition', async () => { + const converterName = 'foo3.js'; + const converterCode = getFileCode('cjs', 'mock-external-converter.js'); + const converterFilePath = path.join(mockBasePath, converterName); - await resetExtension(); - mocksClear.forEach((m) => m.mockClear()); + await resetExtension(); + mocksClear.forEach((m) => m.mockClear()); - //-- SAVE - mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/save', stringify({name: converterName, code: converterCode})); - await flushPromises(); + const errorMsg = `Invalid definition`; + + zhcAddExternalDefinitionSpy.mockImplementationOnce(() => { + throw new Error(errorMsg); + }); - const errorMsg = `Failed to remove definition`; + // await mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/save', stringify({name: converterName, code: converterCode})); + // await flushPromises(); + await getExtension().onMQTTMessage({ + topic: 'zigbee2mqtt/bridge/request/converter/save', + message: {name: converterName, code: converterCode}, + }); - zhcRemoveExternalDefinitionsSpy.mockImplementationOnce(() => { - throw new Error(errorMsg); + expect(mockMQTTPublishAsync).toHaveBeenCalledWith('zigbee2mqtt/bridge/response/converter/save', expect.stringContaining(errorMsg), { + retain: false, + qos: 0, + }); + expect(writeFileSyncSpy).toHaveBeenCalledWith(converterFilePath, converterCode, 'utf8'); + expect(rmSyncSpy).toHaveBeenCalledWith(converterFilePath, {force: true}); }); - //-- REMOVE - mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/remove', stringify({name: converterName})); - await flushPromises(); + it('returns error on failed removal', async () => { + const converterName = 'foo4.js'; + const converterCode = getFileCode('cjs', 'mock-external-converter.js'); + const converterFilePath = path.join(mockBasePath, converterName); - expect(mockMQTTPublishAsync).toHaveBeenCalledWith( - 'zigbee2mqtt/bridge/response/converter/remove', - stringify({data: {}, status: 'error', error: errorMsg}), - {retain: false, qos: 0}, - ); - expect(rmSyncSpy).not.toHaveBeenCalledWith(converterFilePath, {force: true}); - }); + await resetExtension(); + mocksClear.forEach((m) => m.mockClear()); - it('handles invalid payloads', async () => { - await resetExtension(); - mocksClear.forEach((m) => m.mockClear()); + //-- SAVE + // await mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/save', stringify({name: converterName, code: converterCode})); + // await flushPromises(); + await getExtension().onMQTTMessage({ + topic: 'zigbee2mqtt/bridge/request/converter/save', + message: {name: converterName, code: converterCode}, + }); - mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/save', stringify({name: 'test.js', transaction: 1 /* code */})); - await flushPromises(); + const errorMsg = `Failed to remove definition`; - expect(mockMQTTPublishAsync).toHaveBeenCalledWith( - 'zigbee2mqtt/bridge/response/converter/save', - stringify({data: {}, status: 'error', error: `Invalid payload`, transaction: 1}), - {retain: false, qos: 0}, - ); + zhcRemoveExternalDefinitionsSpy.mockImplementationOnce(() => { + throw new Error(errorMsg); + }); - mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/remove', stringify({namex: 'test.js', transaction: 2})); - await flushPromises(); + //-- REMOVE + // await mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/remove', stringify({name: converterName})); + // await flushPromises(); + await getExtension().onMQTTMessage({ + topic: 'zigbee2mqtt/bridge/request/converter/remove', + message: {name: converterName}, + }); + + expect(mockMQTTPublishAsync).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/response/converter/remove', + stringify({data: {}, status: 'error', error: errorMsg}), + {retain: false, qos: 0}, + ); + expect(rmSyncSpy).not.toHaveBeenCalledWith(converterFilePath, {force: true}); + }); + + it('handles invalid payloads', async () => { + await resetExtension(); + mocksClear.forEach((m) => m.mockClear()); + + // await mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/save', stringify({name: 'foo5.js', transaction: 1 /* code */})); + // await flushPromises(); + await getExtension().onMQTTMessage({ + topic: 'zigbee2mqtt/bridge/request/converter/save', + message: {name: 'foo5.js', transaction: 1 /* code */}, + }); + + expect(mockMQTTPublishAsync).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/response/converter/save', + stringify({data: {}, status: 'error', error: `Invalid payload`, transaction: 1}), + {retain: false, qos: 0}, + ); + + // await mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/remove', stringify({namex: 'foo5.js', transaction: 2})); + // await flushPromises(); + await getExtension().onMQTTMessage({ + topic: 'zigbee2mqtt/bridge/request/converter/remove', + message: {namex: 'foo5.js', transaction: 2}, + }); - expect(mockMQTTPublishAsync).toHaveBeenCalledWith( - 'zigbee2mqtt/bridge/response/converter/remove', - stringify({data: {}, status: 'error', error: `Invalid payload`, transaction: 2}), - {retain: false, qos: 0}, - ); + expect(mockMQTTPublishAsync).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/response/converter/remove', + stringify({data: {}, status: 'error', error: `Invalid payload`, transaction: 2}), + {retain: false, qos: 0}, + ); + }); }); }); diff --git a/test/extensions/externalExtensions.test.ts b/test/extensions/externalExtensions.test.ts index 02a3d0b72b..604ba3b9a7 100644 --- a/test/extensions/externalExtensions.test.ts +++ b/test/extensions/externalExtensions.test.ts @@ -1,6 +1,6 @@ import * as data from '../mocks/data'; import {mockLogger} from '../mocks/logger'; -import {mockMQTTEndAsync, events as mockMQTTEvents, mockMQTTPublishAsync} from '../mocks/mqtt'; +import {mockMQTTEndAsync, mockMQTTPublishAsync} from '../mocks/mqtt'; import {flushPromises} from '../mocks/utils'; import {devices, mockController as mockZHController, returnDevices} from '../mocks/zigbeeHerdsman'; @@ -39,12 +39,17 @@ describe('Extension: ExternalExtensions', () => { writeFileSyncSpy, ]; - const useAssets = (): void => { - fs.cpSync(path.join(__dirname, '..', 'assets', BASE_DIR), mockBasePath, {recursive: true}); + const getExtension = (): ExternalExtensions => { + // @ts-expect-error private + return controller.extensions.find((e) => e.constructor.name === 'ExternalExtensions'); }; - const getFileCode = (fileName: string): string => { - return fs.readFileSync(path.join(__dirname, '..', 'assets', BASE_DIR, fileName), 'utf8'); + const useAssets = (mtype: 'cjs' | 'mjs'): void => { + fs.cpSync(path.join(__dirname, '..', 'assets', BASE_DIR, mtype), mockBasePath, {recursive: true}); + }; + + const getFileCode = (mtype: 'cjs' | 'mjs', fileName: string): string => { + return fs.readFileSync(path.join(__dirname, '..', 'assets', BASE_DIR, mtype, fileName), 'utf8'); }; const resetExtension = async (): Promise => { @@ -90,8 +95,8 @@ describe('Extension: ExternalExtensions', () => { expect(mockMQTTPublishAsync).toHaveBeenCalledWith('zigbee2mqtt/bridge/extensions', stringify([]), {retain: true, qos: 0}); }); - it('loads extensions', async () => { - useAssets(); + it('CJS: loads extensions', async () => { + useAssets('cjs'); await controller.start(); await flushPromises(); @@ -103,25 +108,92 @@ describe('Extension: ExternalExtensions', () => { expect(mockMQTTPublishAsync).toHaveBeenCalledWith( 'zigbee2mqtt/bridge/extensions', stringify([ - {name: 'example2Extension.js', code: getFileCode('example2Extension.js')}, - {name: 'exampleExtension.js', code: getFileCode('exampleExtension.js')}, + {name: 'example2Extension.js', code: getFileCode('cjs', 'example2Extension.js')}, + {name: 'exampleExtension.js', code: getFileCode('cjs', 'exampleExtension.js')}, ]), {retain: true, qos: 0}, ); }); - it('saves and removes from MQTT', async () => { - const extensionName = 'foo.js'; - const extensionCode = getFileCode('exampleExtension.js'); - const extensionFilePath = path.join(mockBasePath, extensionName); + it('MJS: loads extensions', async () => { + useAssets('mjs'); await controller.start(); await flushPromises(); + + expect(mockMQTTPublishAsync).toHaveBeenCalledWith('zigbee2mqtt/example2/extension', 'call2 from constructor', {retain: false, qos: 0}); + expect(mockMQTTPublishAsync).toHaveBeenCalledWith('zigbee2mqtt/example2/extension', 'call2 from start', {retain: false, qos: 0}); + expect(mockMQTTPublishAsync).toHaveBeenCalledWith('zigbee2mqtt/example/extension', 'call from constructor', {retain: false, qos: 0}); + expect(mockMQTTPublishAsync).toHaveBeenCalledWith('zigbee2mqtt/example/extension', 'call from start', {retain: false, qos: 0}); + expect(mockMQTTPublishAsync).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/extensions', + stringify([ + {name: 'example2Extension.mjs', code: getFileCode('mjs', 'example2Extension.mjs')}, + {name: 'exampleExtension.mjs', code: getFileCode('mjs', 'exampleExtension.mjs')}, + ]), + {retain: true, qos: 0}, + ); + }); + + it('updates after edit from MQTT', async () => { + const extensionName = 'exampleExtension.js'; + let extensionCode = getFileCode('cjs', 'exampleExtension.js'); + + useAssets('cjs'); + await controller.start(); + await flushPromises(); + + expect(mockMQTTPublishAsync).toHaveBeenCalledWith('zigbee2mqtt/example2/extension', 'call2 from constructor', {retain: false, qos: 0}); + expect(mockMQTTPublishAsync).toHaveBeenCalledWith('zigbee2mqtt/example2/extension', 'call2 from start', {retain: false, qos: 0}); + expect(mockMQTTPublishAsync).toHaveBeenCalledWith('zigbee2mqtt/example/extension', 'call from constructor', {retain: false, qos: 0}); + expect(mockMQTTPublishAsync).toHaveBeenCalledWith('zigbee2mqtt/example/extension', 'call from start', {retain: false, qos: 0}); + expect(mockMQTTPublishAsync).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/extensions', + stringify([ + {name: 'example2Extension.js', code: getFileCode('cjs', 'example2Extension.js')}, + {name: extensionName, code: getFileCode('cjs', extensionName)}, + ]), + {retain: true, qos: 0}, + ); + + extensionCode = extensionCode.replace("'call from start'", "'call from start - edited'"); + + mockMQTTPublishAsync.mockClear(); + await getExtension().onMQTTMessage({ + topic: 'zigbee2mqtt/bridge/request/extension/save', + message: {name: extensionName, code: extensionCode}, + }); + + expect(mockMQTTPublishAsync).toHaveBeenCalledWith('zigbee2mqtt/example/extension', 'call from start - edited', {retain: false, qos: 0}); + + extensionCode = extensionCode.replace("'call from start - edited'", "'call from start'"); + + mockMQTTPublishAsync.mockClear(); + await getExtension().onMQTTMessage({ + topic: 'zigbee2mqtt/bridge/request/extension/save', + message: {name: 'exampleExtension.1.js', code: extensionCode}, + }); + + expect(mockMQTTPublishAsync).toHaveBeenCalledWith('zigbee2mqtt/example/extension', 'call from start', {retain: false, qos: 0}); + }); + }); + + describe('from MQTT', () => { + it('CJS: saves and removes', async () => { + const extensionName = 'foo.js'; + const extensionCode = getFileCode('cjs', 'exampleExtension.js'); + const extensionFilePath = path.join(mockBasePath, extensionName); + + await resetExtension(); mocksClear.forEach((m) => m.mockClear()); //-- SAVE - mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/save', stringify({name: extensionName, code: extensionCode})); - await flushPromises(); + // await mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/save', stringify({name: extensionName, code: extensionCode})); + // await flushPromises(); + await getExtension().onMQTTMessage({ + topic: 'zigbee2mqtt/bridge/request/extension/save', + message: {name: extensionName, code: extensionCode}, + }); expect(mkdirSyncSpy).toHaveBeenCalledWith(mockBasePath, {recursive: true}); expect(writeFileSyncSpy).toHaveBeenCalledWith(extensionFilePath, extensionCode, 'utf8'); @@ -137,72 +209,135 @@ describe('Extension: ExternalExtensions', () => { ); //-- REMOVE - mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/remove', stringify({name: extensionName})); - await flushPromises(); + // await mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/remove', stringify({name: extensionName})); + // await flushPromises(); + await getExtension().onMQTTMessage({ + topic: 'zigbee2mqtt/bridge/request/extension/remove', + message: {name: extensionName}, + }); expect(rmSyncSpy).toHaveBeenCalledWith(extensionFilePath, {force: true}); expect(mockMQTTPublishAsync).toHaveBeenCalledWith('zigbee2mqtt/example/extension', 'call from stop', {retain: false, qos: 0}); expect(mockMQTTPublishAsync).toHaveBeenCalledWith('zigbee2mqtt/bridge/extensions', stringify([]), {retain: true, qos: 0}); }); - }); - it('returns error on invalid code', async () => { - const extensionName = 'foo.js'; - const extensionCode = 'definetly not a correct javascript code'; - const extensionFilePath = path.join(mockBasePath, extensionName); + it('MJS: saves and removes', async () => { + const extensionName = 'foo.mjs'; + const extensionCode = getFileCode('mjs', 'exampleExtension.mjs'); + const extensionFilePath = path.join(mockBasePath, extensionName); - await resetExtension(); - mocksClear.forEach((m) => m.mockClear()); + await resetExtension(); + mocksClear.forEach((m) => m.mockClear()); - mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/save', stringify({name: extensionName, code: extensionCode})); - await flushPromises(); + //-- SAVE + // await mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/save', stringify({name: extensionName, code: extensionCode})); + // await flushPromises(); + await getExtension().onMQTTMessage({ + topic: 'zigbee2mqtt/bridge/request/extension/save', + message: {name: extensionName, code: extensionCode}, + }); - expect(mockMQTTPublishAsync).toHaveBeenCalledWith( - 'zigbee2mqtt/bridge/response/extension/save', - expect.stringContaining(`"error":"${extensionName} contains invalid code`), - {retain: false, qos: 0}, - ); - expect(writeFileSyncSpy).not.toHaveBeenCalledWith(extensionFilePath, extensionCode, 'utf8'); - }); + expect(mkdirSyncSpy).toHaveBeenCalledWith(mockBasePath, {recursive: true}); + expect(writeFileSyncSpy).toHaveBeenCalledWith(extensionFilePath, extensionCode, 'utf8'); + expect(mockMQTTPublishAsync).toHaveBeenCalledWith('zigbee2mqtt/example/extension', 'call from constructor', {retain: false, qos: 0}); + expect(mockMQTTPublishAsync).toHaveBeenCalledWith('zigbee2mqtt/example/extension', 'call from start', {retain: false, qos: 0}); + expect(mockMQTTPublishAsync).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/extensions', + stringify([{name: extensionName, code: extensionCode}]), + { + retain: true, + qos: 0, + }, + ); - it('returns error on invalid removal', async () => { - const converterName = 'invalid.js'; - const converterFilePath = path.join(mockBasePath, converterName); + //-- REMOVE + // await mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/remove', stringify({name: extensionName})); + // await flushPromises(); + await getExtension().onMQTTMessage({ + topic: 'zigbee2mqtt/bridge/request/extension/remove', + message: {name: extensionName}, + }); - await resetExtension(); - mocksClear.forEach((m) => m.mockClear()); + expect(rmSyncSpy).toHaveBeenCalledWith(extensionFilePath, {force: true}); + expect(mockMQTTPublishAsync).toHaveBeenCalledWith('zigbee2mqtt/example/extension', 'call from stop', {retain: false, qos: 0}); + expect(mockMQTTPublishAsync).toHaveBeenCalledWith('zigbee2mqtt/bridge/extensions', stringify([]), {retain: true, qos: 0}); + }); - mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/remove', stringify({name: converterName})); - await flushPromises(); + it('returns error on invalid code', async () => { + const extensionName = 'foo1.js'; + const extensionCode = 'definetly not a correct javascript code'; + const extensionFilePath = path.join(mockBasePath, extensionName); - expect(mockMQTTPublishAsync).toHaveBeenCalledWith( - 'zigbee2mqtt/bridge/response/extension/remove', - stringify({data: {}, status: 'error', error: `${converterName} (${converterFilePath}) doesn't exists`}), - {retain: false, qos: 0}, - ); - expect(rmSyncSpy).not.toHaveBeenCalledWith(converterFilePath, {force: true}); - }); + await resetExtension(); + mocksClear.forEach((m) => m.mockClear()); - it('handles invalid payloads', async () => { - await resetExtension(); - mocksClear.forEach((m) => m.mockClear()); + // await mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/save', stringify({name: extensionName, code: extensionCode})); + // await flushPromises(); + await getExtension().onMQTTMessage({ + topic: 'zigbee2mqtt/bridge/request/extension/save', + message: {name: extensionName, code: extensionCode}, + }); - mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/save', stringify({name: 'test.js', transaction: 1 /* code */})); - await flushPromises(); + expect(mockMQTTPublishAsync).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/response/extension/save', + expect.stringContaining(`"error":"${extensionName} contains invalid code`), + {retain: false, qos: 0}, + ); + expect(writeFileSyncSpy).toHaveBeenCalledWith(extensionFilePath, extensionCode, 'utf8'); + expect(rmSyncSpy).toHaveBeenCalledWith(extensionFilePath, {force: true}); + }); - expect(mockMQTTPublishAsync).toHaveBeenCalledWith( - 'zigbee2mqtt/bridge/response/extension/save', - stringify({data: {}, status: 'error', error: `Invalid payload`, transaction: 1}), - {retain: false, qos: 0}, - ); + it('returns error on invalid removal', async () => { + const extensionName = 'foo2.js'; + const extensionFilePath = path.join(mockBasePath, extensionName); - mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/remove', stringify({namex: 'test.js', transaction: 2})); - await flushPromises(); + await resetExtension(); + mocksClear.forEach((m) => m.mockClear()); - expect(mockMQTTPublishAsync).toHaveBeenCalledWith( - 'zigbee2mqtt/bridge/response/extension/remove', - stringify({data: {}, status: 'error', error: `Invalid payload`, transaction: 2}), - {retain: false, qos: 0}, - ); + // await mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/remove', stringify({name: converterName})); + // await flushPromises(); + await getExtension().onMQTTMessage({ + topic: 'zigbee2mqtt/bridge/request/extension/remove', + message: {name: extensionName}, + }); + + expect(mockMQTTPublishAsync).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/response/extension/remove', + stringify({data: {}, status: 'error', error: `${extensionName} (${extensionFilePath}) doesn't exists`}), + {retain: false, qos: 0}, + ); + expect(rmSyncSpy).not.toHaveBeenCalledWith(extensionFilePath, {force: true}); + }); + + it('handles invalid payloads', async () => { + await resetExtension(); + mocksClear.forEach((m) => m.mockClear()); + + // await mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/save', stringify({name: 'foo3.js', transaction: 1 /* code */})); + // await flushPromises(); + await getExtension().onMQTTMessage({ + topic: 'zigbee2mqtt/bridge/request/extension/save', + message: {name: 'foo3.js', transaction: 1 /* code */}, + }); + + expect(mockMQTTPublishAsync).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/response/extension/save', + stringify({data: {}, status: 'error', error: `Invalid payload`, transaction: 1}), + {retain: false, qos: 0}, + ); + + // await mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/remove', stringify({namex: 'foo3.js', transaction: 2})); + // await flushPromises(); + await getExtension().onMQTTMessage({ + topic: 'zigbee2mqtt/bridge/request/extension/remove', + message: {namex: 'foo3.js', transaction: 2}, + }); + + expect(mockMQTTPublishAsync).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/response/extension/remove', + stringify({data: {}, status: 'error', error: `Invalid payload`, transaction: 2}), + {retain: false, qos: 0}, + ); + }); }); }); diff --git a/test/extensions/networkMap.test.ts b/test/extensions/networkMap.test.ts index ca20010bac..13f377015a 100644 --- a/test/extensions/networkMap.test.ts +++ b/test/extensions/networkMap.test.ts @@ -104,7 +104,7 @@ describe('Extension: NetworkMap', () => { data.writeEmptyState(); fs.mkdirSync(path.join(data.mockDir, 'external_converters')); fs.copyFileSync( - path.join(__dirname, '..', 'assets', 'external_converters', 'mock-external-converter.js'), + path.join(__dirname, '..', 'assets', 'external_converters', 'cjs', 'mock-external-converter.js'), path.join(data.mockDir, 'external_converters', 'mock-external-converter.js'), ); controller = new Controller(vi.fn(), vi.fn()); @@ -302,7 +302,12 @@ describe('Extension: NetworkMap', () => { type: 'Router', }, { - definition: {description: 'external', model: 'external_converter_device', supports: 'linkquality', vendor: 'external'}, + definition: { + description: 'external/converter', + model: 'external_converter_device', + supports: 'linkquality', + vendor: 'external', + }, friendlyName: '0x0017880104e45511', ieeeAddr: '0x0017880104e45511', lastSeen: 1000, @@ -345,7 +350,7 @@ describe('Extension: NetworkMap', () => { "0x0017880104e45525" [style="rounded, filled", fillcolor="#4ea3e0", fontcolor="#ffffff", label="{0x0017880104e45525|0x0017880104e45525 (0x1988)failed: lqi,routingTable|Boef Automatically generated definition (notSupportedModelID)|9 seconds ago}"]; "0x0017880104e45559" [style="rounded, filled", fillcolor="#4ea3e0", fontcolor="#ffffff", label="{cc2530_router|0x0017880104e45559 (0x198c)|Custom devices (DiY) CC2530 router (CC2530.ROUTER)|9 seconds ago}"]; "0x0017880104e45559" -> "0x000b57fffec6a5b2" [penwidth=0.5, weight=0, color="#994444", label="100"] - "0x0017880104e45511" [style="rounded, dashed, filled", fillcolor="#fff8ce", fontcolor="#000000", label="{0x0017880104e45511|0x0017880104e45511 (0x045a)|external external (external_converter_device)|9 seconds ago}"]; + "0x0017880104e45511" [style="rounded, dashed, filled", fillcolor="#fff8ce", fontcolor="#000000", label="{0x0017880104e45511|0x0017880104e45511 (0x045a)|external external/converter (external_converter_device)|9 seconds ago}"]; "0x0017880104e45511" -> "0x00124b00120144ae" [penwidth=1, weight=0, color="#994444", label="92"] }`; @@ -378,7 +383,7 @@ describe('Extension: NetworkMap', () => { --- 0x0017880104e45511 (0x045a) --- - external external (external_converter_device) + external external/converter (external_converter_device) --- 9 seconds ago ] @@ -604,7 +609,12 @@ describe('Extension: NetworkMap', () => { type: 'Router', }, { - definition: {description: 'external', model: 'external_converter_device', supports: 'linkquality', vendor: 'external'}, + definition: { + description: 'external/converter', + model: 'external_converter_device', + supports: 'linkquality', + vendor: 'external', + }, friendlyName: '0x0017880104e45511', ieeeAddr: '0x0017880104e45511', lastSeen: 1000, @@ -756,7 +766,12 @@ describe('Extension: NetworkMap', () => { type: 'Router', }, { - definition: {description: 'external', model: 'external_converter_device', supports: 'linkquality', vendor: 'external'}, + definition: { + description: 'external/converter', + model: 'external_converter_device', + supports: 'linkquality', + vendor: 'external', + }, friendlyName: '0x0017880104e45511', ieeeAddr: '0x0017880104e45511', lastSeen: 1000, From c3cbc29aeb4b10201428ac740dd245bc515979a2 Mon Sep 17 00:00:00 2001 From: Nerivec <62446222+Nerivec@users.noreply.github.com> Date: Thu, 27 Feb 2025 15:08:31 +0100 Subject: [PATCH 2/7] move old versions to only load latest --- lib/extension/externalJS.ts | 12 ++++++++---- test/extensions/externalConverters.test.ts | 20 ++++++++++++++++++-- test/extensions/externalExtensions.test.ts | 20 ++++++++++++++++++-- 3 files changed, 44 insertions(+), 8 deletions(-) diff --git a/lib/extension/externalJS.ts b/lib/extension/externalJS.ts index 6db8b89f6b..6001a7c9f4 100644 --- a/lib/extension/externalJS.ts +++ b/lib/extension/externalJS.ts @@ -13,6 +13,7 @@ import utils from '../util/utils'; import Extension from './extension'; const SUPPORTED_OPERATIONS = ['save', 'remove']; +const BACKUP_DIR = 'old'; export default abstract class ExternalJSExtension extends Extension { protected mqttTopic: string; @@ -45,12 +46,12 @@ export default abstract class ExternalJSExtension extends Extension { await this.publishExternalJS(); } - private getFilePath(name: string, mkBasePath: boolean = false): string { - if (mkBasePath && !fs.existsSync(this.basePath)) { - fs.mkdirSync(this.basePath, {recursive: true}); + private getFilePath(name: string, mkBasePath: boolean = false, backup: boolean = false): string { + if (mkBasePath && !fs.existsSync(backup ? path.join(this.basePath, BACKUP_DIR) : this.basePath)) { + fs.mkdirSync(backup ? path.join(this.basePath, BACKUP_DIR) : this.basePath, {recursive: true}); } - return path.join(this.basePath, name); + return backup ? path.join(this.basePath, BACKUP_DIR, name) : path.join(this.basePath, name); } protected getFileCode(name: string): string { @@ -152,6 +153,9 @@ export default abstract class ExternalJSExtension extends Extension { } newFilePath = this.getFilePath(newName, true); + + // move previous version to backup dir + fs.renameSync(filePath, this.getFilePath(name, true, true)); } try { diff --git a/test/extensions/externalConverters.test.ts b/test/extensions/externalConverters.test.ts index d501c4bd48..024af39f8b 100644 --- a/test/extensions/externalConverters.test.ts +++ b/test/extensions/externalConverters.test.ts @@ -257,7 +257,7 @@ describe('Extension: ExternalConverters', () => { it('updates after edit from MQTT', async () => { const converterName = 'mock-external-converter.js'; - let converterCode = getFileCode('cjs', 'mock-external-converter.js'); + let converterCode = getFileCode('cjs', converterName); useAssets('cjs'); await controller.start(); @@ -273,7 +273,7 @@ describe('Extension: ExternalConverters', () => { 'zigbee2mqtt/bridge/converters', stringify([ {name: 'mock-external-converter-multiple.js', code: getFileCode('cjs', 'mock-external-converter-multiple.js')}, - {name: converterName, code: getFileCode('cjs', converterName)}, + {name: converterName, code: converterCode}, ]), {retain: true, qos: 0}, ); @@ -291,6 +291,14 @@ describe('Extension: ExternalConverters', () => { vendor: 'external', zigbeeModel: ['external_converter_device'], }); + expect(mockMQTTPublishAsync).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/converters', + stringify([ + {name: 'mock-external-converter-multiple.js', code: getFileCode('cjs', 'mock-external-converter-multiple.js')}, + {name: 'mock-external-converter.1.js', code: converterCode}, + ]), + {retain: true, qos: 0}, + ); expect(zhcAddExternalDefinitionSpy).toHaveBeenLastCalledWith( expect.objectContaining({ mock: true, @@ -315,6 +323,14 @@ describe('Extension: ExternalConverters', () => { vendor: 'external', zigbeeModel: ['external_converter_device'], }); + expect(mockMQTTPublishAsync).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/converters', + stringify([ + {name: 'mock-external-converter-multiple.js', code: getFileCode('cjs', 'mock-external-converter-multiple.js')}, + {name: 'mock-external-converter.2.js', code: converterCode}, + ]), + {retain: true, qos: 0}, + ); expect(zhcAddExternalDefinitionSpy).toHaveBeenLastCalledWith( expect.objectContaining({ mock: true, diff --git a/test/extensions/externalExtensions.test.ts b/test/extensions/externalExtensions.test.ts index 604ba3b9a7..341ee2e20a 100644 --- a/test/extensions/externalExtensions.test.ts +++ b/test/extensions/externalExtensions.test.ts @@ -137,7 +137,7 @@ describe('Extension: ExternalExtensions', () => { it('updates after edit from MQTT', async () => { const extensionName = 'exampleExtension.js'; - let extensionCode = getFileCode('cjs', 'exampleExtension.js'); + let extensionCode = getFileCode('cjs', extensionName); useAssets('cjs'); await controller.start(); @@ -151,7 +151,7 @@ describe('Extension: ExternalExtensions', () => { 'zigbee2mqtt/bridge/extensions', stringify([ {name: 'example2Extension.js', code: getFileCode('cjs', 'example2Extension.js')}, - {name: extensionName, code: getFileCode('cjs', extensionName)}, + {name: extensionName, code: extensionCode}, ]), {retain: true, qos: 0}, ); @@ -165,6 +165,14 @@ describe('Extension: ExternalExtensions', () => { }); expect(mockMQTTPublishAsync).toHaveBeenCalledWith('zigbee2mqtt/example/extension', 'call from start - edited', {retain: false, qos: 0}); + expect(mockMQTTPublishAsync).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/extensions', + stringify([ + {name: 'example2Extension.js', code: getFileCode('cjs', 'example2Extension.js')}, + {name: 'exampleExtension.1.js', code: extensionCode}, + ]), + {retain: true, qos: 0}, + ); extensionCode = extensionCode.replace("'call from start - edited'", "'call from start'"); @@ -175,6 +183,14 @@ describe('Extension: ExternalExtensions', () => { }); expect(mockMQTTPublishAsync).toHaveBeenCalledWith('zigbee2mqtt/example/extension', 'call from start', {retain: false, qos: 0}); + expect(mockMQTTPublishAsync).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/extensions', + stringify([ + {name: 'example2Extension.js', code: getFileCode('cjs', 'example2Extension.js')}, + {name: 'exampleExtension.2.js', code: extensionCode}, + ]), + {retain: true, qos: 0}, + ); }); }); From d7494b37eeaaad437b561c58edf00e3ecfad1ed4 Mon Sep 17 00:00:00 2001 From: Nerivec <62446222+Nerivec@users.noreply.github.com> Date: Sat, 15 Mar 2025 15:23:48 +0100 Subject: [PATCH 3/7] fixes --- test/extensions/externalConverters.test.ts | 54 ++++++---------------- test/extensions/externalExtensions.test.ts | 41 ++++------------ 2 files changed, 23 insertions(+), 72 deletions(-) diff --git a/test/extensions/externalConverters.test.ts b/test/extensions/externalConverters.test.ts index 024af39f8b..5753b60dfb 100644 --- a/test/extensions/externalConverters.test.ts +++ b/test/extensions/externalConverters.test.ts @@ -4,7 +4,6 @@ import {mockMQTTEndAsync, mockMQTTPublishAsync} from '../mocks/mqtt'; import {flushPromises} from '../mocks/utils'; import {devices, mockController as mockZHController, returnDevices} from '../mocks/zigbeeHerdsman'; -import type ExternalConverters from '../../lib/extension/externalConverters'; import type Device from '../../lib/model/device'; import fs from 'node:fs'; @@ -49,11 +48,6 @@ describe('Extension: ExternalConverters', () => { zhcRemoveExternalDefinitionsSpy, ]; - const getExtension = (): ExternalConverters => { - // @ts-expect-error private - return controller.extensions.find((e) => e.constructor.name === 'ExternalConverters'); - }; - const useAssets = (mtype: 'cjs' | 'mjs'): void => { fs.cpSync(path.join(__dirname, '..', 'assets', BASE_DIR, mtype), mockBasePath, {recursive: true}); }; @@ -280,7 +274,7 @@ describe('Extension: ExternalConverters', () => { converterCode = converterCode.replace("posix.join('external', 'converter')", "posix.join('external', 'converter', 'edited')"); - await getExtension().onMQTTMessage({ + await (controller.getExtension('ExternalConverters')! as ExternalConverters).onMQTTMessage({ topic: 'zigbee2mqtt/bridge/request/converter/save', message: {name: converterName, code: converterCode}, }); @@ -312,7 +306,7 @@ describe('Extension: ExternalConverters', () => { converterCode = converterCode.replace("posix.join('external', 'converter', 'edited')", "posix.join('external', 'converter')"); - await getExtension().onMQTTMessage({ + await (controller.getExtension('ExternalConverters')! as ExternalConverters).onMQTTMessage({ topic: 'zigbee2mqtt/bridge/request/converter/save', message: {name: 'mock-external-converter.1.js', code: converterCode}, }); @@ -361,9 +355,7 @@ describe('Extension: ExternalConverters', () => { }); //-- SAVE - // await mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/save', stringify({name: converterName, code: converterCode})); - // await flushPromises(); - await getExtension().onMQTTMessage({ + await (controller.getExtension('ExternalConverters')! as ExternalConverters).onMQTTMessage({ topic: 'zigbee2mqtt/bridge/request/converter/save', message: {name: converterName, code: converterCode}, }); @@ -397,9 +389,7 @@ describe('Extension: ExternalConverters', () => { ); //-- REMOVE - // await mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/remove', stringify({name: converterName})); - // await flushPromises(); - await getExtension().onMQTTMessage({ + await (controller.getExtension('ExternalConverters')! as ExternalConverters).onMQTTMessage({ topic: 'zigbee2mqtt/bridge/request/converter/remove', message: {name: converterName}, }); @@ -432,9 +422,7 @@ describe('Extension: ExternalConverters', () => { }); //-- SAVE - // await mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/save', stringify({name: converterName, code: converterCode})); - // await flushPromises(); - await getExtension().onMQTTMessage({ + await (controller.getExtension('ExternalConverters')! as ExternalConverters).onMQTTMessage({ topic: 'zigbee2mqtt/bridge/request/converter/save', message: {name: converterName, code: converterCode}, }); @@ -468,9 +456,7 @@ describe('Extension: ExternalConverters', () => { ); //-- REMOVE - // await mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/remove', stringify({name: converterName})); - // await flushPromises(); - await getExtension().onMQTTMessage({ + await (controller.getExtension('ExternalConverters')! as ExternalConverters).onMQTTMessage({ topic: 'zigbee2mqtt/bridge/request/converter/remove', message: {name: converterName}, }); @@ -495,9 +481,7 @@ describe('Extension: ExternalConverters', () => { await resetExtension(); mocksClear.forEach((m) => m.mockClear()); - // await mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/save', stringify({name: converterName, code: converterCode})); - // await flushPromises(); - await getExtension().onMQTTMessage({ + await (controller.getExtension('ExternalConverters')! as ExternalConverters).onMQTTMessage({ topic: 'zigbee2mqtt/bridge/request/converter/save', message: {name: converterName, code: converterCode}, }); @@ -518,9 +502,7 @@ describe('Extension: ExternalConverters', () => { await resetExtension(); mocksClear.forEach((m) => m.mockClear()); - // await mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/remove', stringify({name: converterName})); - // await flushPromises(); - await getExtension().onMQTTMessage({ + await (controller.getExtension('ExternalConverters')! as ExternalConverters).onMQTTMessage({ topic: 'zigbee2mqtt/bridge/request/converter/remove', message: {name: converterName}, }); @@ -547,9 +529,7 @@ describe('Extension: ExternalConverters', () => { throw new Error(errorMsg); }); - // await mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/save', stringify({name: converterName, code: converterCode})); - // await flushPromises(); - await getExtension().onMQTTMessage({ + await (controller.getExtension('ExternalConverters')! as ExternalConverters).onMQTTMessage({ topic: 'zigbee2mqtt/bridge/request/converter/save', message: {name: converterName, code: converterCode}, }); @@ -571,9 +551,7 @@ describe('Extension: ExternalConverters', () => { mocksClear.forEach((m) => m.mockClear()); //-- SAVE - // await mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/save', stringify({name: converterName, code: converterCode})); - // await flushPromises(); - await getExtension().onMQTTMessage({ + await (controller.getExtension('ExternalConverters')! as ExternalConverters).onMQTTMessage({ topic: 'zigbee2mqtt/bridge/request/converter/save', message: {name: converterName, code: converterCode}, }); @@ -585,9 +563,7 @@ describe('Extension: ExternalConverters', () => { }); //-- REMOVE - // await mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/remove', stringify({name: converterName})); - // await flushPromises(); - await getExtension().onMQTTMessage({ + await (controller.getExtension('ExternalConverters')! as ExternalConverters).onMQTTMessage({ topic: 'zigbee2mqtt/bridge/request/converter/remove', message: {name: converterName}, }); @@ -604,9 +580,7 @@ describe('Extension: ExternalConverters', () => { await resetExtension(); mocksClear.forEach((m) => m.mockClear()); - // await mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/save', stringify({name: 'foo5.js', transaction: 1 /* code */})); - // await flushPromises(); - await getExtension().onMQTTMessage({ + await (controller.getExtension('ExternalConverters')! as ExternalConverters).onMQTTMessage({ topic: 'zigbee2mqtt/bridge/request/converter/save', message: {name: 'foo5.js', transaction: 1 /* code */}, }); @@ -617,9 +591,7 @@ describe('Extension: ExternalConverters', () => { {retain: false, qos: 0}, ); - // await mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/remove', stringify({namex: 'foo5.js', transaction: 2})); - // await flushPromises(); - await getExtension().onMQTTMessage({ + await (controller.getExtension('ExternalConverters')! as ExternalConverters).onMQTTMessage({ topic: 'zigbee2mqtt/bridge/request/converter/remove', message: {namex: 'foo5.js', transaction: 2}, }); diff --git a/test/extensions/externalExtensions.test.ts b/test/extensions/externalExtensions.test.ts index 341ee2e20a..e43f10ccf8 100644 --- a/test/extensions/externalExtensions.test.ts +++ b/test/extensions/externalExtensions.test.ts @@ -39,11 +39,6 @@ describe('Extension: ExternalExtensions', () => { writeFileSyncSpy, ]; - const getExtension = (): ExternalExtensions => { - // @ts-expect-error private - return controller.extensions.find((e) => e.constructor.name === 'ExternalExtensions'); - }; - const useAssets = (mtype: 'cjs' | 'mjs'): void => { fs.cpSync(path.join(__dirname, '..', 'assets', BASE_DIR, mtype), mockBasePath, {recursive: true}); }; @@ -159,7 +154,7 @@ describe('Extension: ExternalExtensions', () => { extensionCode = extensionCode.replace("'call from start'", "'call from start - edited'"); mockMQTTPublishAsync.mockClear(); - await getExtension().onMQTTMessage({ + await (controller.getExtension('ExternalExtensions')! as ExternalExtensions).onMQTTMessage({ topic: 'zigbee2mqtt/bridge/request/extension/save', message: {name: extensionName, code: extensionCode}, }); @@ -177,7 +172,7 @@ describe('Extension: ExternalExtensions', () => { extensionCode = extensionCode.replace("'call from start - edited'", "'call from start'"); mockMQTTPublishAsync.mockClear(); - await getExtension().onMQTTMessage({ + await (controller.getExtension('ExternalExtensions')! as ExternalExtensions).onMQTTMessage({ topic: 'zigbee2mqtt/bridge/request/extension/save', message: {name: 'exampleExtension.1.js', code: extensionCode}, }); @@ -204,9 +199,7 @@ describe('Extension: ExternalExtensions', () => { mocksClear.forEach((m) => m.mockClear()); //-- SAVE - // await mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/save', stringify({name: extensionName, code: extensionCode})); - // await flushPromises(); - await getExtension().onMQTTMessage({ + await (controller.getExtension('ExternalExtensions')! as ExternalExtensions).onMQTTMessage({ topic: 'zigbee2mqtt/bridge/request/extension/save', message: {name: extensionName, code: extensionCode}, }); @@ -225,9 +218,7 @@ describe('Extension: ExternalExtensions', () => { ); //-- REMOVE - // await mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/remove', stringify({name: extensionName})); - // await flushPromises(); - await getExtension().onMQTTMessage({ + await (controller.getExtension('ExternalExtensions')! as ExternalExtensions).onMQTTMessage({ topic: 'zigbee2mqtt/bridge/request/extension/remove', message: {name: extensionName}, }); @@ -246,9 +237,7 @@ describe('Extension: ExternalExtensions', () => { mocksClear.forEach((m) => m.mockClear()); //-- SAVE - // await mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/save', stringify({name: extensionName, code: extensionCode})); - // await flushPromises(); - await getExtension().onMQTTMessage({ + await (controller.getExtension('ExternalExtensions')! as ExternalExtensions).onMQTTMessage({ topic: 'zigbee2mqtt/bridge/request/extension/save', message: {name: extensionName, code: extensionCode}, }); @@ -267,9 +256,7 @@ describe('Extension: ExternalExtensions', () => { ); //-- REMOVE - // await mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/remove', stringify({name: extensionName})); - // await flushPromises(); - await getExtension().onMQTTMessage({ + await (controller.getExtension('ExternalExtensions')! as ExternalExtensions).onMQTTMessage({ topic: 'zigbee2mqtt/bridge/request/extension/remove', message: {name: extensionName}, }); @@ -287,9 +274,7 @@ describe('Extension: ExternalExtensions', () => { await resetExtension(); mocksClear.forEach((m) => m.mockClear()); - // await mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/save', stringify({name: extensionName, code: extensionCode})); - // await flushPromises(); - await getExtension().onMQTTMessage({ + await (controller.getExtension('ExternalExtensions')! as ExternalExtensions).onMQTTMessage({ topic: 'zigbee2mqtt/bridge/request/extension/save', message: {name: extensionName, code: extensionCode}, }); @@ -310,9 +295,7 @@ describe('Extension: ExternalExtensions', () => { await resetExtension(); mocksClear.forEach((m) => m.mockClear()); - // await mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/remove', stringify({name: converterName})); - // await flushPromises(); - await getExtension().onMQTTMessage({ + await (controller.getExtension('ExternalExtensions')! as ExternalExtensions).onMQTTMessage({ topic: 'zigbee2mqtt/bridge/request/extension/remove', message: {name: extensionName}, }); @@ -329,9 +312,7 @@ describe('Extension: ExternalExtensions', () => { await resetExtension(); mocksClear.forEach((m) => m.mockClear()); - // await mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/save', stringify({name: 'foo3.js', transaction: 1 /* code */})); - // await flushPromises(); - await getExtension().onMQTTMessage({ + await (controller.getExtension('ExternalExtensions')! as ExternalExtensions).onMQTTMessage({ topic: 'zigbee2mqtt/bridge/request/extension/save', message: {name: 'foo3.js', transaction: 1 /* code */}, }); @@ -342,9 +323,7 @@ describe('Extension: ExternalExtensions', () => { {retain: false, qos: 0}, ); - // await mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/remove', stringify({namex: 'foo3.js', transaction: 2})); - // await flushPromises(); - await getExtension().onMQTTMessage({ + await (controller.getExtension('ExternalExtensions')! as ExternalExtensions).onMQTTMessage({ topic: 'zigbee2mqtt/bridge/request/extension/remove', message: {namex: 'foo3.js', transaction: 2}, }); From 6e0c35cc6f423a5d6647019357f45869f67e9079 Mon Sep 17 00:00:00 2001 From: Nerivec <62446222+Nerivec@users.noreply.github.com> Date: Sat, 15 Mar 2025 16:01:01 +0100 Subject: [PATCH 4/7] cleanup --- test/assets/external_extensions/mjs/example2Extension.mjs | 4 +--- test/assets/external_extensions/mjs/exampleExtension.mjs | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/test/assets/external_extensions/mjs/example2Extension.mjs b/test/assets/external_extensions/mjs/example2Extension.mjs index 4ac629db47..576c7b61db 100644 --- a/test/assets/external_extensions/mjs/example2Extension.mjs +++ b/test/assets/external_extensions/mjs/example2Extension.mjs @@ -1,4 +1,4 @@ -class Example2 { +export default class Example2 { constructor(zigbee, mqtt, state, publishEntityState, eventBus) { this.mqtt = mqtt; this.mqtt.publish('example2/extension', 'call2 from constructor'); @@ -12,5 +12,3 @@ class Example2 { this.mqtt.publish('example/extension', 'call2 from stop'); } } - -export default Example2; diff --git a/test/assets/external_extensions/mjs/exampleExtension.mjs b/test/assets/external_extensions/mjs/exampleExtension.mjs index 40e373f78f..76a37123e1 100644 --- a/test/assets/external_extensions/mjs/exampleExtension.mjs +++ b/test/assets/external_extensions/mjs/exampleExtension.mjs @@ -1,4 +1,4 @@ -class Example { +export default class Example { constructor(zigbee, mqtt, state, publishEntityState, eventBus) { this.mqtt = mqtt; this.mqtt.publish('example/extension', 'call from constructor'); @@ -12,5 +12,3 @@ class Example { this.mqtt.publish('example/extension', 'call from stop'); } } - -export default Example; From 2001426fe39e8c8aac60e27fd3a5db2bd4bc350d Mon Sep 17 00:00:00 2001 From: Nerivec <62446222+Nerivec@users.noreply.github.com> Date: Mon, 17 Mar 2025 19:50:31 +0100 Subject: [PATCH 5/7] fix: improve error handling (fixes Koenkk/zigbee2mqtt#26763) --- lib/extension/externalConverters.ts | 7 ++-- lib/extension/externalExtensions.ts | 48 +++++++++++++--------- lib/extension/externalJS.ts | 15 ++++++- test/extensions/externalConverters.test.ts | 22 ++++++++++ test/extensions/externalExtensions.test.ts | 22 ++++++++++ 5 files changed, 90 insertions(+), 24 deletions(-) diff --git a/lib/extension/externalConverters.ts b/lib/extension/externalConverters.ts index fe92f7844a..89c91b929a 100644 --- a/lib/extension/externalConverters.ts +++ b/lib/extension/externalConverters.ts @@ -54,10 +54,11 @@ export default class ExternalConverters extends ExternalJSExtension { await this.zigbee.resolveDevicesDefinitions(true); } catch (error) { - /* v8 ignore next */ - logger.error(`Failed to load external converter '${newName ?? name}'`); - logger.error(`Check the code for syntax error and make sure it is up to date with the current Zigbee2MQTT version.`); logger.error( + /* v8 ignore next */ + `Failed to load external converter '${newName ?? name}'. Check the code for syntax error and make sure it is up to date with the current Zigbee2MQTT version.`, + ); + logger.warning( `External converters are not meant for long term usage, but for local testing after which a pull request should be created to add out-of-the-box support for the device`, ); diff --git a/lib/extension/externalExtensions.ts b/lib/extension/externalExtensions.ts index e5baedba59..90c120cbbe 100644 --- a/lib/extension/externalExtensions.ts +++ b/lib/extension/externalExtensions.ts @@ -36,25 +36,35 @@ export default class ExternalExtensions extends ExternalJSExtension { } protected async loadJS(name: string, mod: TModule, newName?: string): Promise { - // stop if already started - await this.enableDisableExtension(false, mod.name); - await this.addExtension( - new mod( - this.zigbee, - this.mqtt, - this.state, - this.publishEntityState, - this.eventBus, - this.enableDisableExtension, - this.restartCallback, - this.addExtension, - // @ts-expect-error additional params that don't fit the internal `Extension` type - settings, - logger, - ), - ); + try { + // stop if already started + await this.enableDisableExtension(false, mod.name); + await this.addExtension( + new mod( + this.zigbee, + this.mqtt, + this.state, + this.publishEntityState, + this.eventBus, + this.enableDisableExtension, + this.restartCallback, + this.addExtension, + // @ts-expect-error additional params that don't fit the internal `Extension` type + settings, + logger, + ), + ); + + /* v8 ignore start */ + logger.info(`Loaded external extension '${newName ?? name}'.`); + } catch (error) { + logger.error( + /* v8 ignore next */ + `Failed to load external extension '${newName ?? name}'. Check the code for syntax error and make sure it is up to date with the current Zigbee2MQTT version.`, + ); - /* v8 ignore next */ - logger.info(`Loaded external extension '${newName ?? name}'.`); + throw error; + } + /* v8 ignore stop */ } } diff --git a/lib/extension/externalJS.ts b/lib/extension/externalJS.ts index 6001a7c9f4..0a5c4e7eea 100644 --- a/lib/extension/externalJS.ts +++ b/lib/extension/externalJS.ts @@ -177,9 +177,20 @@ export default abstract class ExternalJSExtension extends Extension { private async loadFiles(): Promise { for (const extension of this.getFiles()) { - const mod = await import(path.join(this.basePath, extension.name)); + try { + const mod = await import(path.join(this.basePath, extension.name)); + + await this.loadJS(extension.name, mod.default); + } catch (error) { + const destPath = this.getFilePath(extension.name, true, true); - await this.loadJS(extension.name, mod.default); + fs.renameSync(this.getFilePath(extension.name), destPath); + + logger.error( + `Invalid external ${this.mqttTopic} '${extension.name}' was moved to '${destPath}' to prevent interference with Zigbee2MQTT.`, + ); + logger.debug((error as Error).stack!); + } } } diff --git a/test/extensions/externalConverters.test.ts b/test/extensions/externalConverters.test.ts index 5753b60dfb..6bfd4a9af9 100644 --- a/test/extensions/externalConverters.test.ts +++ b/test/extensions/externalConverters.test.ts @@ -336,6 +336,28 @@ describe('Extension: ExternalConverters', () => { }), ); }); + + it('loads all valid converters, relocates & skips ones with errors', async () => { + useAssets('mjs'); + + const filepath = path.join(mockBasePath, 'invalid.mjs'); + + fs.writeFileSync(filepath, 'invalid js', 'utf8'); + + await controller.start(); + await flushPromises(); + + expect(mockMQTTPublishAsync).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/converters', + stringify([ + {name: 'mock-external-converter-multiple.mjs', code: getFileCode('mjs', 'mock-external-converter-multiple.mjs')}, + {name: 'mock-external-converter.mjs', code: getFileCode('mjs', 'mock-external-converter.mjs')}, + ]), + {retain: true, qos: 0}, + ); + expect(fs.existsSync(filepath)).toStrictEqual(false); + expect(fs.existsSync(path.join(mockBasePath, 'old', 'invalid.mjs'))).toStrictEqual(true); + }); }); describe('from MQTT', () => { diff --git a/test/extensions/externalExtensions.test.ts b/test/extensions/externalExtensions.test.ts index e43f10ccf8..d441c1871a 100644 --- a/test/extensions/externalExtensions.test.ts +++ b/test/extensions/externalExtensions.test.ts @@ -130,6 +130,28 @@ describe('Extension: ExternalExtensions', () => { ); }); + it('loads all valid extensions, relocates & skips ones with errors', async () => { + useAssets('mjs'); + + const filepath = path.join(mockBasePath, 'invalid.mjs'); + + fs.writeFileSync(filepath, 'invalid js', 'utf8'); + + await controller.start(); + await flushPromises(); + + expect(mockMQTTPublishAsync).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/extensions', + stringify([ + {name: 'example2Extension.mjs', code: getFileCode('mjs', 'example2Extension.mjs')}, + {name: 'exampleExtension.mjs', code: getFileCode('mjs', 'exampleExtension.mjs')}, + ]), + {retain: true, qos: 0}, + ); + expect(fs.existsSync(filepath)).toStrictEqual(false); + expect(fs.existsSync(path.join(mockBasePath, 'old', 'invalid.mjs'))).toStrictEqual(true); + }); + it('updates after edit from MQTT', async () => { const extensionName = 'exampleExtension.js'; let extensionCode = getFileCode('cjs', extensionName); From c878baf665cf681599a7c44e2fa4a9d35ee3a7aa Mon Sep 17 00:00:00 2001 From: Nerivec <62446222+Nerivec@users.noreply.github.com> Date: Tue, 18 Mar 2025 20:47:34 +0100 Subject: [PATCH 6/7] updated approach --- lib/extension/externalJS.ts | 87 ++++++++++++------- .../mjs/mock-external-converter.mjs | 6 +- test/extensions/externalConverters.test.ts | 41 +++------ test/extensions/externalExtensions.test.ts | 38 +++----- 4 files changed, 88 insertions(+), 84 deletions(-) diff --git a/lib/extension/externalJS.ts b/lib/extension/externalJS.ts index 0a5c4e7eea..f13b648d7c 100644 --- a/lib/extension/externalJS.ts +++ b/lib/extension/externalJS.ts @@ -13,12 +13,13 @@ import utils from '../util/utils'; import Extension from './extension'; const SUPPORTED_OPERATIONS = ['save', 'remove']; -const BACKUP_DIR = 'old'; export default abstract class ExternalJSExtension extends Extension { + protected folderName: string; protected mqttTopic: string; protected requestRegex: RegExp; protected basePath: string; + protected srcBasePath: string; constructor( zigbee: Zigbee, @@ -34,9 +35,12 @@ export default abstract class ExternalJSExtension extends Extension { ) { super(zigbee, mqtt, state, publishEntityState, eventBus, enableDisableExtension, restartCallback, addExtension); + this.folderName = folderName; this.mqttTopic = mqttTopic; this.requestRegex = new RegExp(`${settings.get().mqtt.base_topic}/bridge/request/${mqttTopic}/(save|remove)`); this.basePath = data.joinPath(folderName); + // 1-up from this file + this.srcBasePath = path.join(__dirname, '..', folderName); } override async start(): Promise { @@ -46,24 +50,34 @@ export default abstract class ExternalJSExtension extends Extension { await this.publishExternalJS(); } - private getFilePath(name: string, mkBasePath: boolean = false, backup: boolean = false): string { - if (mkBasePath && !fs.existsSync(backup ? path.join(this.basePath, BACKUP_DIR) : this.basePath)) { - fs.mkdirSync(backup ? path.join(this.basePath, BACKUP_DIR) : this.basePath, {recursive: true}); + override async stop(): Promise { + // remove src base path on stop to ensure always back to default + fs.rmSync(this.srcBasePath, {force: true, recursive: true}); + await super.stop(); + } + + private getFilePath(name: string, mkBasePath = false, inSource = false): string { + const basePath = inSource ? this.srcBasePath : this.basePath; + + if (mkBasePath && !fs.existsSync(basePath)) { + fs.mkdirSync(basePath, {recursive: true}); } - return backup ? path.join(this.basePath, BACKUP_DIR, name) : path.join(this.basePath, name); + return path.join(basePath, name); } protected getFileCode(name: string): string { - return fs.readFileSync(path.join(this.basePath, name), 'utf8'); + return fs.readFileSync(this.getFilePath(name), 'utf8'); } - protected *getFiles(): Generator<{name: string; code: string}> { - if (!fs.existsSync(this.basePath)) { + protected *getFiles(inSource = false): Generator<{name: string; code: string}> { + const basePath = inSource ? this.srcBasePath : this.basePath; + + if (!fs.existsSync(basePath)) { return; } - for (const fileName of fs.readdirSync(this.basePath)) { + for (const fileName of fs.readdirSync(basePath)) { if (fileName.endsWith('.js') || fileName.endsWith('.cjs') || fileName.endsWith('.mjs')) { yield {name: fileName, code: this.getFileCode(fileName)}; } @@ -112,19 +126,21 @@ export default abstract class ExternalJSExtension extends Extension { } const {name} = message; + const srcToBeRemoved = this.getFilePath(name, false, true); const toBeRemoved = this.getFilePath(name); - if (fs.existsSync(toBeRemoved)) { - const mod = await import(toBeRemoved); + if (fs.existsSync(srcToBeRemoved)) { + const mod = await import(this.getImportPath(srcToBeRemoved)); await this.removeJS(name, mod.default); + fs.rmSync(srcToBeRemoved, {force: true}); fs.rmSync(toBeRemoved, {force: true}); logger.info(`${name} (${toBeRemoved}) removed.`); await this.publishExternalJS(); return utils.getResponse(message, {}); } else { - return utils.getResponse(message, {}, `${name} (${toBeRemoved}) doesn't exists`); + return utils.getResponse(message, {}, `${name} (${srcToBeRemoved}) doesn't exists`); } } @@ -136,11 +152,10 @@ export default abstract class ExternalJSExtension extends Extension { } const {name, code} = message; - const filePath = this.getFilePath(name, true); - let newFilePath = filePath; + const srcFilePath = this.getFilePath(name, true, true); let newName = name; - if (fs.existsSync(filePath)) { + if (fs.existsSync(srcFilePath)) { // if file already exist, version it to bypass node module caching const versionMatch = name.match(/\.(\d+)\.(c|m)?js$/); @@ -152,24 +167,28 @@ export default abstract class ExternalJSExtension extends Extension { newName = name.replace(ext, `.1${ext}`); } - newFilePath = this.getFilePath(newName, true); - - // move previous version to backup dir - fs.renameSync(filePath, this.getFilePath(name, true, true)); + // remove previous version + fs.rmSync(srcFilePath, {force: true}); + fs.rmSync(this.getFilePath(name, true, false), {force: true}); } + const newSrcFilePath = this.getFilePath(newName, false /* already created above if needed */, true); + try { - fs.writeFileSync(newFilePath, code, 'utf8'); + fs.writeFileSync(newSrcFilePath, code, 'utf8'); - const mod = await import(newFilePath); + const mod = await import(this.getImportPath(newSrcFilePath)); await this.loadJS(name, mod.default, newName); - logger.info(`${newName} loaded. Contents written to '${newFilePath}'.`); + logger.info(`${newName} loaded. Contents written to '${newSrcFilePath}'.`); + // keep original in data folder synced + fs.writeFileSync(this.getFilePath(newName, true, false), code, 'utf8'); await this.publishExternalJS(); return utils.getResponse(message, {}); } catch (error) { - fs.rmSync(newFilePath, {force: true}); + fs.rmSync(newSrcFilePath, {force: true}); + // NOTE: original in data folder doesn't get written if invalid return utils.getResponse(message, {}, `${newName} contains invalid code: ${(error as Error).message}`); } @@ -177,17 +196,22 @@ export default abstract class ExternalJSExtension extends Extension { private async loadFiles(): Promise { for (const extension of this.getFiles()) { + const srcFilePath = this.getFilePath(extension.name, true, true); + const filePath = this.getFilePath(extension.name); + try { - const mod = await import(path.join(this.basePath, extension.name)); + fs.copyFileSync(filePath, srcFilePath); + + const mod = await import(this.getImportPath(srcFilePath)); await this.loadJS(extension.name, mod.default); } catch (error) { - const destPath = this.getFilePath(extension.name, true, true); - - fs.renameSync(this.getFilePath(extension.name), destPath); + // change ext so Z2M doesn't try to load it again and again + fs.renameSync(filePath, `${filePath}.invalid`); + fs.rmSync(srcFilePath, {force: true}); logger.error( - `Invalid external ${this.mqttTopic} '${extension.name}' was moved to '${destPath}' to prevent interference with Zigbee2MQTT.`, + `Invalid external ${this.mqttTopic} '${extension.name}' was ignored and renamed to prevent interference with Zigbee2MQTT.`, ); logger.debug((error as Error).stack!); } @@ -197,7 +221,7 @@ export default abstract class ExternalJSExtension extends Extension { private async publishExternalJS(): Promise { await this.mqtt.publish( `bridge/${this.mqttTopic}s`, - stringify(Array.from(this.getFiles())), + stringify(Array.from(this.getFiles(true))), { retain: true, qos: 0, @@ -206,4 +230,9 @@ export default abstract class ExternalJSExtension extends Extension { true, ); } + + private getImportPath(filePath: string): string { + // prevent issues on Windows + return path.relative(__dirname, filePath).replaceAll('\\', '/'); + } } diff --git a/test/assets/external_converters/mjs/mock-external-converter.mjs b/test/assets/external_converters/mjs/mock-external-converter.mjs index 099e9c62a0..ec7e341937 100644 --- a/test/assets/external_converters/mjs/mock-external-converter.mjs +++ b/test/assets/external_converters/mjs/mock-external-converter.mjs @@ -1,12 +1,12 @@ import {posix} from 'node:path'; +import {identify} from 'zigbee-herdsman-converters/lib/modernExtend'; + export default { mock: true, zigbeeModel: ['external_converter_device'], vendor: 'external', model: 'external_converter_device', description: posix.join('external', 'converter'), - fromZigbee: [], - toZigbee: [], - exposes: [], + extend: [identify()], }; diff --git a/test/extensions/externalConverters.test.ts b/test/extensions/externalConverters.test.ts index 6bfd4a9af9..b8318a9132 100644 --- a/test/extensions/externalConverters.test.ts +++ b/test/extensions/externalConverters.test.ts @@ -23,9 +23,6 @@ describe('Extension: ExternalConverters', () => { const mockBasePath = path.join(data.mockDir, BASE_DIR); let controller: Controller; - const existsSyncSpy = vi.spyOn(fs, 'existsSync'); - const readdirSyncSpy = vi.spyOn(fs, 'readdirSync'); - const mkdirSyncSpy = vi.spyOn(fs, 'mkdirSync'); const rmSyncSpy = vi.spyOn(fs, 'rmSync'); const writeFileSyncSpy = vi.spyOn(fs, 'writeFileSync'); @@ -39,9 +36,6 @@ describe('Extension: ExternalConverters', () => { mockLogger.error, mockZHController.stop, devices.bulb.save, - existsSyncSpy, - readdirSyncSpy, - mkdirSyncSpy, rmSyncSpy, writeFileSyncSpy, zhcAddExternalDefinitionSpy, @@ -93,6 +87,7 @@ describe('Extension: ExternalConverters', () => { fs.rmSync(mockBasePath, {recursive: true, force: true}); await controller?.stop(); + await flushPromises(); }); describe('from folder', () => { @@ -104,8 +99,6 @@ describe('Extension: ExternalConverters', () => { await controller.start(); await flushPromises(); - expect(existsSyncSpy).toHaveBeenCalledWith(mockBasePath); - expect(readdirSyncSpy).not.toHaveBeenCalledWith(mockBasePath); expect(mockMQTTPublishAsync).toHaveBeenCalledWith('zigbee2mqtt/bridge/converters', stringify([]), {retain: true, qos: 0}); }); @@ -356,7 +349,7 @@ describe('Extension: ExternalConverters', () => { {retain: true, qos: 0}, ); expect(fs.existsSync(filepath)).toStrictEqual(false); - expect(fs.existsSync(path.join(mockBasePath, 'old', 'invalid.mjs'))).toStrictEqual(true); + expect(fs.existsSync(path.join(mockBasePath, 'invalid.mjs.invalid'))).toStrictEqual(true); }); }); @@ -364,7 +357,6 @@ describe('Extension: ExternalConverters', () => { it('CJS: saves and removes', async () => { const converterName = 'foo.js'; const converterCode = getFileCode('cjs', 'mock-external-converter.js'); - const converterFilePath = path.join(mockBasePath, converterName); await resetExtension(); mocksClear.forEach((m) => m.mockClear()); @@ -388,8 +380,7 @@ describe('Extension: ExternalConverters', () => { vendor: 'external', zigbeeModel: ['external_converter_device'], }); - expect(mkdirSyncSpy).toHaveBeenCalledWith(mockBasePath, {recursive: true}); - expect(writeFileSyncSpy).toHaveBeenCalledWith(converterFilePath, converterCode, 'utf8'); + expect(writeFileSyncSpy).toHaveBeenCalledWith(expect.stringContaining(converterName), converterCode, 'utf8'); expect(zhcRemoveExternalDefinitionsSpy).toHaveBeenCalledTimes(1); expect(zhcRemoveExternalDefinitionsSpy).toHaveBeenNthCalledWith(1, converterName); expect(zhcAddExternalDefinitionSpy).toHaveBeenCalledWith( @@ -422,7 +413,7 @@ describe('Extension: ExternalConverters', () => { vendor: '', zigbeeModel: ['external_converter_device'], }); - expect(rmSyncSpy).toHaveBeenCalledWith(converterFilePath, {force: true}); + expect(rmSyncSpy).toHaveBeenCalledWith(expect.stringContaining(converterName), {force: true}); expect(zhcRemoveExternalDefinitionsSpy).toHaveBeenCalledTimes(2); expect(zhcRemoveExternalDefinitionsSpy).toHaveBeenNthCalledWith(2, converterName); expect(mockMQTTPublishAsync).toHaveBeenCalledWith('zigbee2mqtt/bridge/converters', stringify([]), {retain: true, qos: 0}); @@ -431,7 +422,6 @@ describe('Extension: ExternalConverters', () => { it('MJS: saves and removes', async () => { const converterName = 'foo.mjs'; const converterCode = getFileCode('mjs', 'mock-external-converter.mjs'); - const converterFilePath = path.join(mockBasePath, converterName); await resetExtension(); mocksClear.forEach((m) => m.mockClear()); @@ -455,8 +445,7 @@ describe('Extension: ExternalConverters', () => { vendor: 'external', zigbeeModel: ['external_converter_device'], }); - expect(mkdirSyncSpy).toHaveBeenCalledWith(mockBasePath, {recursive: true}); - expect(writeFileSyncSpy).toHaveBeenCalledWith(converterFilePath, converterCode, 'utf8'); + expect(writeFileSyncSpy).toHaveBeenCalledWith(expect.stringContaining(converterName), converterCode, 'utf8'); expect(zhcRemoveExternalDefinitionsSpy).toHaveBeenCalledTimes(1); expect(zhcRemoveExternalDefinitionsSpy).toHaveBeenNthCalledWith(1, converterName); expect(zhcAddExternalDefinitionSpy).toHaveBeenCalledWith( @@ -489,7 +478,7 @@ describe('Extension: ExternalConverters', () => { vendor: '', zigbeeModel: ['external_converter_device'], }); - expect(rmSyncSpy).toHaveBeenCalledWith(converterFilePath, {force: true}); + expect(rmSyncSpy).toHaveBeenCalledWith(expect.stringContaining(converterName), {force: true}); expect(zhcRemoveExternalDefinitionsSpy).toHaveBeenCalledTimes(2); expect(zhcRemoveExternalDefinitionsSpy).toHaveBeenNthCalledWith(2, converterName); expect(mockMQTTPublishAsync).toHaveBeenCalledWith('zigbee2mqtt/bridge/converters', stringify([]), {retain: true, qos: 0}); @@ -498,7 +487,6 @@ describe('Extension: ExternalConverters', () => { it('returns error on invalid code', async () => { const converterName = 'foo1.js'; const converterCode = 'definetly not a correct javascript code'; - const converterFilePath = path.join(mockBasePath, converterName); await resetExtension(); mocksClear.forEach((m) => m.mockClear()); @@ -513,13 +501,12 @@ describe('Extension: ExternalConverters', () => { expect.stringContaining(`"error":"${converterName} contains invalid code`), {retain: false, qos: 0}, ); - expect(writeFileSyncSpy).toHaveBeenCalledWith(converterFilePath, converterCode, 'utf8'); - expect(rmSyncSpy).toHaveBeenCalledWith(converterFilePath, {force: true}); + expect(writeFileSyncSpy).toHaveBeenCalledWith(expect.stringContaining(converterName), converterCode, 'utf8'); + expect(rmSyncSpy).toHaveBeenCalledWith(expect.stringContaining(converterName), {force: true}); }); it('returns error on invalid removal', async () => { const converterName = 'foo2.js'; - const converterFilePath = path.join(mockBasePath, converterName); await resetExtension(); mocksClear.forEach((m) => m.mockClear()); @@ -531,16 +518,15 @@ describe('Extension: ExternalConverters', () => { expect(mockMQTTPublishAsync).toHaveBeenCalledWith( 'zigbee2mqtt/bridge/response/converter/remove', - stringify({data: {}, status: 'error', error: `${converterName} (${converterFilePath}) doesn't exists`}), + expect.stringContaining("doesn't exists"), {retain: false, qos: 0}, ); - expect(rmSyncSpy).not.toHaveBeenCalledWith(converterFilePath, {force: true}); + expect(rmSyncSpy).not.toHaveBeenCalledWith(expect.stringContaining(converterName), {force: true}); }); it('returns error on invalid definition', async () => { const converterName = 'foo3.js'; const converterCode = getFileCode('cjs', 'mock-external-converter.js'); - const converterFilePath = path.join(mockBasePath, converterName); await resetExtension(); mocksClear.forEach((m) => m.mockClear()); @@ -560,14 +546,13 @@ describe('Extension: ExternalConverters', () => { retain: false, qos: 0, }); - expect(writeFileSyncSpy).toHaveBeenCalledWith(converterFilePath, converterCode, 'utf8'); - expect(rmSyncSpy).toHaveBeenCalledWith(converterFilePath, {force: true}); + expect(writeFileSyncSpy).toHaveBeenCalledWith(expect.stringContaining(converterName), converterCode, 'utf8'); + expect(rmSyncSpy).toHaveBeenCalledWith(expect.stringContaining(converterName), {force: true}); }); it('returns error on failed removal', async () => { const converterName = 'foo4.js'; const converterCode = getFileCode('cjs', 'mock-external-converter.js'); - const converterFilePath = path.join(mockBasePath, converterName); await resetExtension(); mocksClear.forEach((m) => m.mockClear()); @@ -595,7 +580,7 @@ describe('Extension: ExternalConverters', () => { stringify({data: {}, status: 'error', error: errorMsg}), {retain: false, qos: 0}, ); - expect(rmSyncSpy).not.toHaveBeenCalledWith(converterFilePath, {force: true}); + expect(rmSyncSpy).not.toHaveBeenCalledWith(expect.stringContaining(converterName), {force: true}); }); it('handles invalid payloads', async () => { diff --git a/test/extensions/externalExtensions.test.ts b/test/extensions/externalExtensions.test.ts index d441c1871a..077bd32d9c 100644 --- a/test/extensions/externalExtensions.test.ts +++ b/test/extensions/externalExtensions.test.ts @@ -19,9 +19,6 @@ describe('Extension: ExternalExtensions', () => { let controller: Controller; const mockBasePath = path.join(data.mockDir, BASE_DIR); - const existsSyncSpy = vi.spyOn(fs, 'existsSync'); - const readdirSyncSpy = vi.spyOn(fs, 'readdirSync'); - const mkdirSyncSpy = vi.spyOn(fs, 'mkdirSync'); const rmSyncSpy = vi.spyOn(fs, 'rmSync'); const writeFileSyncSpy = vi.spyOn(fs, 'writeFileSync'); @@ -32,9 +29,6 @@ describe('Extension: ExternalExtensions', () => { mockLogger.error, mockZHController.stop, devices.bulb.save, - existsSyncSpy, - readdirSyncSpy, - mkdirSyncSpy, rmSyncSpy, writeFileSyncSpy, ]; @@ -56,6 +50,7 @@ describe('Extension: ExternalExtensions', () => { vi.useFakeTimers(); controller = new Controller(vi.fn(), vi.fn()); + await controller.start(); await flushPromises(); }); @@ -72,8 +67,11 @@ describe('Extension: ExternalExtensions', () => { returnDevices.splice(0); }); - afterEach(() => { + afterEach(async () => { fs.rmSync(mockBasePath, {recursive: true, force: true}); + + await controller?.stop(); + await flushPromises(); }); describe('from folder', () => { @@ -85,8 +83,6 @@ describe('Extension: ExternalExtensions', () => { await controller.start(); await flushPromises(); - expect(existsSyncSpy).toHaveBeenCalledWith(mockBasePath); - expect(readdirSyncSpy).not.toHaveBeenCalledWith(mockBasePath); expect(mockMQTTPublishAsync).toHaveBeenCalledWith('zigbee2mqtt/bridge/extensions', stringify([]), {retain: true, qos: 0}); }); @@ -149,7 +145,7 @@ describe('Extension: ExternalExtensions', () => { {retain: true, qos: 0}, ); expect(fs.existsSync(filepath)).toStrictEqual(false); - expect(fs.existsSync(path.join(mockBasePath, 'old', 'invalid.mjs'))).toStrictEqual(true); + expect(fs.existsSync(path.join(mockBasePath, 'invalid.mjs.invalid'))).toStrictEqual(true); }); it('updates after edit from MQTT', async () => { @@ -215,7 +211,6 @@ describe('Extension: ExternalExtensions', () => { it('CJS: saves and removes', async () => { const extensionName = 'foo.js'; const extensionCode = getFileCode('cjs', 'exampleExtension.js'); - const extensionFilePath = path.join(mockBasePath, extensionName); await resetExtension(); mocksClear.forEach((m) => m.mockClear()); @@ -226,8 +221,7 @@ describe('Extension: ExternalExtensions', () => { message: {name: extensionName, code: extensionCode}, }); - expect(mkdirSyncSpy).toHaveBeenCalledWith(mockBasePath, {recursive: true}); - expect(writeFileSyncSpy).toHaveBeenCalledWith(extensionFilePath, extensionCode, 'utf8'); + expect(writeFileSyncSpy).toHaveBeenCalledWith(expect.stringContaining(extensionName), extensionCode, 'utf8'); expect(mockMQTTPublishAsync).toHaveBeenCalledWith('zigbee2mqtt/example/extension', 'call from constructor', {retain: false, qos: 0}); expect(mockMQTTPublishAsync).toHaveBeenCalledWith('zigbee2mqtt/example/extension', 'call from start', {retain: false, qos: 0}); expect(mockMQTTPublishAsync).toHaveBeenCalledWith( @@ -245,7 +239,7 @@ describe('Extension: ExternalExtensions', () => { message: {name: extensionName}, }); - expect(rmSyncSpy).toHaveBeenCalledWith(extensionFilePath, {force: true}); + expect(rmSyncSpy).toHaveBeenCalledWith(expect.stringContaining(extensionName), {force: true}); expect(mockMQTTPublishAsync).toHaveBeenCalledWith('zigbee2mqtt/example/extension', 'call from stop', {retain: false, qos: 0}); expect(mockMQTTPublishAsync).toHaveBeenCalledWith('zigbee2mqtt/bridge/extensions', stringify([]), {retain: true, qos: 0}); }); @@ -253,7 +247,6 @@ describe('Extension: ExternalExtensions', () => { it('MJS: saves and removes', async () => { const extensionName = 'foo.mjs'; const extensionCode = getFileCode('mjs', 'exampleExtension.mjs'); - const extensionFilePath = path.join(mockBasePath, extensionName); await resetExtension(); mocksClear.forEach((m) => m.mockClear()); @@ -264,8 +257,7 @@ describe('Extension: ExternalExtensions', () => { message: {name: extensionName, code: extensionCode}, }); - expect(mkdirSyncSpy).toHaveBeenCalledWith(mockBasePath, {recursive: true}); - expect(writeFileSyncSpy).toHaveBeenCalledWith(extensionFilePath, extensionCode, 'utf8'); + expect(writeFileSyncSpy).toHaveBeenCalledWith(expect.stringContaining(extensionName), extensionCode, 'utf8'); expect(mockMQTTPublishAsync).toHaveBeenCalledWith('zigbee2mqtt/example/extension', 'call from constructor', {retain: false, qos: 0}); expect(mockMQTTPublishAsync).toHaveBeenCalledWith('zigbee2mqtt/example/extension', 'call from start', {retain: false, qos: 0}); expect(mockMQTTPublishAsync).toHaveBeenCalledWith( @@ -283,7 +275,7 @@ describe('Extension: ExternalExtensions', () => { message: {name: extensionName}, }); - expect(rmSyncSpy).toHaveBeenCalledWith(extensionFilePath, {force: true}); + expect(rmSyncSpy).toHaveBeenCalledWith(expect.stringContaining(extensionName), {force: true}); expect(mockMQTTPublishAsync).toHaveBeenCalledWith('zigbee2mqtt/example/extension', 'call from stop', {retain: false, qos: 0}); expect(mockMQTTPublishAsync).toHaveBeenCalledWith('zigbee2mqtt/bridge/extensions', stringify([]), {retain: true, qos: 0}); }); @@ -291,7 +283,6 @@ describe('Extension: ExternalExtensions', () => { it('returns error on invalid code', async () => { const extensionName = 'foo1.js'; const extensionCode = 'definetly not a correct javascript code'; - const extensionFilePath = path.join(mockBasePath, extensionName); await resetExtension(); mocksClear.forEach((m) => m.mockClear()); @@ -306,13 +297,12 @@ describe('Extension: ExternalExtensions', () => { expect.stringContaining(`"error":"${extensionName} contains invalid code`), {retain: false, qos: 0}, ); - expect(writeFileSyncSpy).toHaveBeenCalledWith(extensionFilePath, extensionCode, 'utf8'); - expect(rmSyncSpy).toHaveBeenCalledWith(extensionFilePath, {force: true}); + expect(writeFileSyncSpy).toHaveBeenCalledWith(expect.stringContaining(extensionName), extensionCode, 'utf8'); + expect(rmSyncSpy).toHaveBeenCalledWith(expect.stringContaining(extensionName), {force: true}); }); it('returns error on invalid removal', async () => { const extensionName = 'foo2.js'; - const extensionFilePath = path.join(mockBasePath, extensionName); await resetExtension(); mocksClear.forEach((m) => m.mockClear()); @@ -324,10 +314,10 @@ describe('Extension: ExternalExtensions', () => { expect(mockMQTTPublishAsync).toHaveBeenCalledWith( 'zigbee2mqtt/bridge/response/extension/remove', - stringify({data: {}, status: 'error', error: `${extensionName} (${extensionFilePath}) doesn't exists`}), + expect.stringContaining("doesn't exists"), {retain: false, qos: 0}, ); - expect(rmSyncSpy).not.toHaveBeenCalledWith(extensionFilePath, {force: true}); + expect(rmSyncSpy).not.toHaveBeenCalledWith(expect.stringContaining(extensionName), {force: true}); }); it('handles invalid payloads', async () => { From b4883a3215fca594541351deb542a3dde753b2b2 Mon Sep 17 00:00:00 2001 From: Nerivec <62446222+Nerivec@users.noreply.github.com> Date: Tue, 18 Mar 2025 21:35:35 +0100 Subject: [PATCH 7/7] prevent race in vitest with files being manipulated from same location --- lib/extension/externalJS.ts | 7 ++++++- test/controller.test.ts | 1 + test/extensions/availability.test.ts | 3 ++- test/extensions/bind.test.ts | 2 ++ test/extensions/bridge.test.ts | 2 ++ test/extensions/configure.test.ts | 2 ++ test/extensions/frontend.test.ts | 2 ++ test/extensions/groups.test.ts | 2 ++ test/extensions/homeassistant.test.ts | 4 +++- test/extensions/networkMap.test.ts | 2 ++ test/extensions/onEvent.test.ts | 2 ++ test/extensions/otaUpdate.test.ts | 2 ++ test/extensions/publish.test.ts | 4 +++- test/extensions/receive.test.ts | 4 +++- 14 files changed, 34 insertions(+), 5 deletions(-) diff --git a/lib/extension/externalJS.ts b/lib/extension/externalJS.ts index f13b648d7c..5cbbcaebc7 100644 --- a/lib/extension/externalJS.ts +++ b/lib/extension/externalJS.ts @@ -40,7 +40,12 @@ export default abstract class ExternalJSExtension extends Extension { this.requestRegex = new RegExp(`${settings.get().mqtt.base_topic}/bridge/request/${mqttTopic}/(save|remove)`); this.basePath = data.joinPath(folderName); // 1-up from this file - this.srcBasePath = path.join(__dirname, '..', folderName); + this.srcBasePath = path.join( + __dirname, + '..', + // prevent race in vitest with files being manipulated from same location + process.env.VITEST_WORKER_ID ? /* v8 ignore next */ `${folderName}_${Math.floor(Math.random() * 10000)}` : folderName, + ); } override async start(): Promise { diff --git a/test/controller.test.ts b/test/controller.test.ts index 95c1bd518c..1267fe3706 100644 --- a/test/controller.test.ts +++ b/test/controller.test.ts @@ -75,6 +75,7 @@ describe('Controller', () => { afterEach(async () => { await controller?.stop(); + await flushPromises(); }); it('Start controller', async () => { diff --git a/test/extensions/availability.test.ts b/test/extensions/availability.test.ts index 490146109f..2c509d2db9 100644 --- a/test/extensions/availability.test.ts +++ b/test/extensions/availability.test.ts @@ -66,7 +66,8 @@ describe('Extension: Availability', () => { afterEach(async () => {}); afterAll(async () => { - await controller.stop(); + await controller?.stop(); + await flushPromises(); vi.useRealTimers(); }); diff --git a/test/extensions/bind.test.ts b/test/extensions/bind.test.ts index 6d26570d72..90ce5910d1 100644 --- a/test/extensions/bind.test.ts +++ b/test/extensions/bind.test.ts @@ -54,6 +54,8 @@ describe('Extension: Bind', () => { }); afterAll(async () => { + await controller?.stop(); + await flushPromises(); vi.useRealTimers(); }); diff --git a/test/extensions/bridge.test.ts b/test/extensions/bridge.test.ts index 70e34af716..07c2c19a56 100644 --- a/test/extensions/bridge.test.ts +++ b/test/extensions/bridge.test.ts @@ -83,6 +83,8 @@ describe('Extension: Bridge', () => { }); afterAll(async () => { + await controller?.stop(); + await flushPromises(); vi.useRealTimers(); }); diff --git a/test/extensions/configure.test.ts b/test/extensions/configure.test.ts index 7eca6d2e95..ace2042bf2 100644 --- a/test/extensions/configure.test.ts +++ b/test/extensions/configure.test.ts @@ -82,6 +82,8 @@ describe('Extension: Configure', () => { }); afterAll(async () => { + await controller?.stop(); + await flushPromises(); vi.useRealTimers(); }); diff --git a/test/extensions/frontend.test.ts b/test/extensions/frontend.test.ts index c5cb07d199..1a68082ef0 100644 --- a/test/extensions/frontend.test.ts +++ b/test/extensions/frontend.test.ts @@ -154,6 +154,8 @@ describe('Extension: Frontend', () => { afterEach(async () => { delete devices.bulb.linkquality; + await controller?.stop(); + await flushPromises(); }); it('Start/stop with defaults', async () => { diff --git a/test/extensions/groups.test.ts b/test/extensions/groups.test.ts index 2f09f1835a..fe7458fae7 100644 --- a/test/extensions/groups.test.ts +++ b/test/extensions/groups.test.ts @@ -33,6 +33,8 @@ describe('Extension: Groups', () => { }); afterAll(async () => { + await controller?.stop(); + await flushPromises(); vi.useRealTimers(); }); diff --git a/test/extensions/homeassistant.test.ts b/test/extensions/homeassistant.test.ts index 01c04637ac..16df21675b 100644 --- a/test/extensions/homeassistant.test.ts +++ b/test/extensions/homeassistant.test.ts @@ -64,8 +64,10 @@ describe('Extension: HomeAssistant', () => { }); afterAll(async () => { - vi.useRealTimers(); mockSleep.restore(); + await controller?.stop(); + await flushPromises(); + vi.useRealTimers(); }); beforeEach(async () => { diff --git a/test/extensions/networkMap.test.ts b/test/extensions/networkMap.test.ts index 13f377015a..d7ea2ef1fb 100644 --- a/test/extensions/networkMap.test.ts +++ b/test/extensions/networkMap.test.ts @@ -124,6 +124,8 @@ describe('Extension: NetworkMap', () => { afterAll(async () => { mockSleep.restore(); fs.rmSync(path.join(data.mockDir, 'external_converters'), {recursive: true}); + await controller?.stop(); + await flushPromises(); vi.useRealTimers(); }); diff --git a/test/extensions/onEvent.test.ts b/test/extensions/onEvent.test.ts index 6890c8d04d..d929e6fb2f 100644 --- a/test/extensions/onEvent.test.ts +++ b/test/extensions/onEvent.test.ts @@ -41,6 +41,8 @@ describe('Extension: OnEvent', () => { }); afterAll(async () => { + await controller?.stop(); + await flushPromises(); vi.useRealTimers(); }); diff --git a/test/extensions/otaUpdate.test.ts b/test/extensions/otaUpdate.test.ts index be527844af..a6a1084d51 100644 --- a/test/extensions/otaUpdate.test.ts +++ b/test/extensions/otaUpdate.test.ts @@ -46,6 +46,8 @@ describe('Extension: OTAUpdate', () => { afterAll(async () => { mockSleep.restore(); + await controller?.stop(); + await flushPromises(); vi.useRealTimers(); }); diff --git a/test/extensions/publish.test.ts b/test/extensions/publish.test.ts index 668f08a2e7..ad43ca4764 100644 --- a/test/extensions/publish.test.ts +++ b/test/extensions/publish.test.ts @@ -63,8 +63,10 @@ describe('Extension: Publish', () => { afterAll(async () => { await vi.runOnlyPendingTimersAsync(); - vi.useRealTimers(); mockSleep.restore(); + await controller?.stop(); + await flushPromises(); + vi.useRealTimers(); }); it('Should publish messages to zigbee devices', async () => { diff --git a/test/extensions/receive.test.ts b/test/extensions/receive.test.ts index 261b07e4f2..9a8cd84390 100644 --- a/test/extensions/receive.test.ts +++ b/test/extensions/receive.test.ts @@ -33,8 +33,10 @@ describe('Extension: Receive', () => { }); afterAll(async () => { - vi.useRealTimers(); mockSleep.restore(); + await controller?.stop(); + await flushPromises(); + vi.useRealTimers(); }); it('Should handle a zigbee message', async () => {