From b0c527a4562d2c46c9606ef9ffbdcf9acad3f635 Mon Sep 17 00:00:00 2001 From: Nerivec <62446222+Nerivec@users.noreply.github.com> Date: Wed, 13 Nov 2024 00:26:32 +0100 Subject: [PATCH 01/26] feat: Live loading of external JS (converters/extensions) --- lib/controller.ts | 11 +- lib/extension/externalConverters.ts | 56 ++++++---- lib/extension/externalExtension.ts | 120 --------------------- lib/extension/externalExtensions.ts | 59 ++++++++++ lib/extension/externalJS.ts | 162 ++++++++++++++++++++++++++++ lib/util/utils.ts | 47 -------- 6 files changed, 263 insertions(+), 192 deletions(-) delete mode 100644 lib/extension/externalExtension.ts create mode 100644 lib/extension/externalExtensions.ts create mode 100644 lib/extension/externalJS.ts diff --git a/lib/controller.ts b/lib/controller.ts index 8d5ecde8ca..ef308b8713 100644 --- a/lib/controller.ts +++ b/lib/controller.ts @@ -14,7 +14,7 @@ import ExtensionBind from './extension/bind'; import ExtensionBridge from './extension/bridge'; import ExtensionConfigure from './extension/configure'; import ExtensionExternalConverters from './extension/externalConverters'; -import ExtensionExternalExtension from './extension/externalExtension'; +import ExtensionExternalExtensions from './extension/externalExtensions'; // Extensions import ExtensionFrontend from './extension/frontend'; import ExtensionGroups from './extension/groups'; @@ -46,7 +46,7 @@ const AllExtensions = [ ExtensionOTAUpdate, ExtensionExternalConverters, ExtensionFrontend, - ExtensionExternalExtension, + ExtensionExternalExtensions, ExtensionAvailability, ]; @@ -105,7 +105,8 @@ export class Controller { new ExtensionGroups(...this.extensionArgs), new ExtensionBind(...this.extensionArgs), new ExtensionOTAUpdate(...this.extensionArgs), - new ExtensionExternalExtension(...this.extensionArgs), + new ExtensionExternalExtensions(...this.extensionArgs), + new ExtensionExternalConverters(...this.extensionArgs), new ExtensionAvailability(...this.extensionArgs), ]; @@ -113,10 +114,6 @@ export class Controller { this.extensions.push(new ExtensionFrontend(...this.extensionArgs)); } - if (settings.get().external_converters.length) { - this.extensions.push(new ExtensionExternalConverters(...this.extensionArgs)); - } - if (settings.get().homeassistant) { this.extensions.push(new ExtensionHomeAssistant(...this.extensionArgs)); } diff --git a/lib/extension/externalConverters.ts b/lib/extension/externalConverters.ts index 06d4e77892..910b1cdf64 100644 --- a/lib/extension/externalConverters.ts +++ b/lib/extension/externalConverters.ts @@ -1,11 +1,11 @@ -import * as zhc from 'zigbee-herdsman-converters'; +import {addDefinition, removeDefinition} from 'zigbee-herdsman-converters'; import logger from '../util/logger'; -import * as settings from '../util/settings'; -import {loadExternalConverter} from '../util/utils'; -import Extension from './extension'; +import ExternalJSExtension from './externalJS'; -export default class ExternalConverters extends Extension { +type ModuleExports = ExternalDefinition | ExternalDefinition[]; + +export default class ExternalConverters extends ExternalJSExtension { constructor( zigbee: Zigbee, mqtt: MQTT, @@ -16,25 +16,45 @@ export default class ExternalConverters extends Extension { restartCallback: () => Promise, addExtension: (extension: Extension) => Promise, ) { - super(zigbee, mqtt, state, publishEntityState, eventBus, enableDisableExtension, restartCallback, addExtension); + super( + zigbee, + mqtt, + state, + publishEntityState, + eventBus, + enableDisableExtension, + restartCallback, + addExtension, + 'converter', + 'external_converters', + ); + } + + protected async removeJS(name: string, module: ModuleExports): Promise { + for (const definition of this.getDefinitions(module)) { + // TODO: implement in ZHC + removeDefinition(definition); + } + } - for (const file of settings.get().external_converters) { + protected async loadJS(name: string, module: ModuleExports): Promise { + for (const definition of this.getDefinitions(module)) { try { - for (const definition of loadExternalConverter(file)) { - zhc.addDefinition(definition); - } - logger.info(`Loaded external converter '${file}'`); + // TODO: `updateDefinition` in ZHC instead? (add if not exist, replace if exist) + removeDefinition(definition); + addDefinition(definition); + logger.info(`Loaded external converter '${name}'.`); } catch (error) { - logger.error(`Failed to load external converter file '${file}' (${(error as Error).message})`); + logger.error(`Failed to load external converter '${name}' (${(error as Error).message})`); + logger.error(`Check the code for syntax error and make sure it is up to date with the current Zigbee2MQTT version.`); logger.error( - `Probably there is a syntax error in the file or the external converter is not ` + - `compatible with the current Zigbee2MQTT version`, - ); - logger.error( - `Note that external converters are not meant for long term usage, it's meant for local ` + - `testing after which a pull request should be created to add out-of-the-box support for the device`, + `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`, ); } } } + + private getDefinitions(module: ModuleExports): ExternalDefinition[] { + return Array.isArray(module) ? module : [module]; + } } diff --git a/lib/extension/externalExtension.ts b/lib/extension/externalExtension.ts deleted file mode 100644 index 6df6568554..0000000000 --- a/lib/extension/externalExtension.ts +++ /dev/null @@ -1,120 +0,0 @@ -import fs from 'fs'; -import path from 'path'; - -import bind from 'bind-decorator'; -import stringify from 'json-stable-stringify-without-jsonify'; - -import * as settings from '../util/settings'; -import utils from '../util/utils'; -import data from './../util/data'; -import logger from './../util/logger'; -import Extension from './extension'; - -const requestRegex = new RegExp(`${settings.get().mqtt.base_topic}/bridge/request/extension/(save|remove)`); - -export default class ExternalExtension extends Extension { - private requestLookup: {[s: string]: (message: KeyValue) => Promise} = { - save: this.saveExtension, - remove: this.removeExtension, - }; - - override async start(): Promise { - this.eventBus.onMQTTMessage(this, this.onMQTTMessage); - await this.loadUserDefinedExtensions(); - await this.publishExtensions(); - } - - private getExtensionsBasePath(): string { - return data.joinPath('extension'); - } - - private getListOfUserDefinedExtensions(): {name: string; code: string}[] { - const basePath = this.getExtensionsBasePath(); - if (fs.existsSync(basePath)) { - return fs - .readdirSync(basePath) - .filter((f) => f.endsWith('.js')) - .map((fileName) => { - const extensionFilePath = path.join(basePath, fileName); - return {name: fileName, code: fs.readFileSync(extensionFilePath, 'utf-8')}; - }); - } else { - return []; - } - } - - @bind private async removeExtension(message: KeyValue): Promise { - const {name} = message; - const extensions = this.getListOfUserDefinedExtensions(); - const extensionToBeRemoved = extensions.find((e) => e.name === name); - - if (extensionToBeRemoved) { - await this.enableDisableExtension(false, extensionToBeRemoved.name); - const basePath = this.getExtensionsBasePath(); - const extensionFilePath = path.join(basePath, path.basename(name)); - fs.unlinkSync(extensionFilePath); - await this.publishExtensions(); - logger.info(`Extension ${name} removed`); - return utils.getResponse(message, {}); - } else { - return utils.getResponse(message, {}, `Extension ${name} doesn't exists`); - } - } - - @bind private async saveExtension(message: KeyValue): Promise { - const {name, code} = message; - const ModuleConstructor = utils.loadModuleFromText(code, name) as typeof Extension; - await this.loadExtension(ModuleConstructor); - const basePath = this.getExtensionsBasePath(); - /* istanbul ignore else */ - if (!fs.existsSync(basePath)) { - fs.mkdirSync(basePath); - } - const extensionFilePath = path.join(basePath, path.basename(name)); - fs.writeFileSync(extensionFilePath, code); - await this.publishExtensions(); - logger.info(`Extension ${name} loaded`); - return utils.getResponse(message, {}); - } - - @bind async onMQTTMessage(data: eventdata.MQTTMessage): Promise { - const match = data.topic.match(requestRegex); - if (match && this.requestLookup[match[1].toLowerCase()]) { - const message = utils.parseJSON(data.message, data.message) as KeyValue; - try { - const response = await this.requestLookup[match[1].toLowerCase()](message); - await this.mqtt.publish(`bridge/response/extension/${match[1]}`, stringify(response)); - } catch (error) { - logger.error(`Request '${data.topic}' failed with error: '${(error as Error).message}'`); - const response = utils.getResponse(message, {}, `${(error as Error).message}`); - await this.mqtt.publish(`bridge/response/extension/${match[1]}`, stringify(response)); - } - } - } - - @bind private async loadExtension(ConstructorClass: typeof Extension): Promise { - await this.enableDisableExtension(false, ConstructorClass.name); - // @ts-expect-error `ConstructorClass` is the interface, not the actual passed class - await this.addExtension(new ConstructorClass(this.zigbee, this.mqtt, this.state, this.publishEntityState, this.eventBus, settings, logger)); - } - - private async loadUserDefinedExtensions(): Promise { - for (const extension of this.getListOfUserDefinedExtensions()) { - await this.loadExtension(utils.loadModuleFromText(extension.code, extension.name) as typeof Extension); - } - } - - private async publishExtensions(): Promise { - const extensions = this.getListOfUserDefinedExtensions(); - await this.mqtt.publish( - 'bridge/extensions', - stringify(extensions), - { - retain: true, - qos: 0, - }, - settings.get().mqtt.base_topic, - true, - ); - } -} diff --git a/lib/extension/externalExtensions.ts b/lib/extension/externalExtensions.ts new file mode 100644 index 0000000000..3537617b34 --- /dev/null +++ b/lib/extension/externalExtensions.ts @@ -0,0 +1,59 @@ +import type Extension from './extension'; + +import logger from '../util/logger'; +import * as settings from '../util/settings'; +import ExternalJSExtension from './externalJS'; + +type ModuleExports = typeof Extension; + +export default class ExternalExtensions extends ExternalJSExtension { + constructor( + zigbee: Zigbee, + mqtt: MQTT, + state: State, + publishEntityState: PublishEntityState, + eventBus: EventBus, + enableDisableExtension: (enable: boolean, name: string) => Promise, + restartCallback: () => Promise, + addExtension: (extension: Extension) => Promise, + ) { + super( + zigbee, + mqtt, + state, + publishEntityState, + eventBus, + enableDisableExtension, + restartCallback, + addExtension, + 'extension', + 'external_extensions', + ); + } + + protected async removeJS(name: string, module: ModuleExports): Promise { + await this.enableDisableExtension(false, module.name); + } + + protected async loadJS(name: string, module: ModuleExports): Promise { + // stop if already started + await this.enableDisableExtension(false, module.name); + await this.addExtension( + // @ts-expect-error `module` is the interface, not the actual passed class + new module( + this.zigbee, + this.mqtt, + this.state, + this.publishEntityState, + this.eventBus, + this.enableDisableExtension, + this.restartCallback, + this.addExtension, + settings, + logger, + ), + ); + + logger.info(`Loaded external extension '${name}'.`); + } +} diff --git a/lib/extension/externalJS.ts b/lib/extension/externalJS.ts new file mode 100644 index 0000000000..f496804a6a --- /dev/null +++ b/lib/extension/externalJS.ts @@ -0,0 +1,162 @@ +import fs from 'fs'; +import path from 'path'; +import {Context, runInNewContext} from 'vm'; + +import bind from 'bind-decorator'; +import stringify from 'json-stable-stringify-without-jsonify'; +import data from 'lib/util/data'; +import utils from 'lib/util/utils'; + +import * as settings from '../util/settings'; +import logger from './../util/logger'; +import Extension from './extension'; + +export default abstract class ExternalJSExtension extends Extension { + private requestLookup: {[s: string]: (message: KeyValue) => Promise} = { + save: this.save, + remove: this.remove, + }; + + protected mqttTopic: string; + protected requestRegex: RegExp; + protected basePath: string; + + constructor( + zigbee: Zigbee, + mqtt: MQTT, + state: State, + publishEntityState: PublishEntityState, + eventBus: EventBus, + enableDisableExtension: (enable: boolean, name: string) => Promise, + restartCallback: () => Promise, + addExtension: (extension: Extension) => Promise, + mqttTopic: string, + folderName: string, + ) { + super(zigbee, mqtt, state, publishEntityState, eventBus, enableDisableExtension, restartCallback, addExtension); + + this.mqttTopic = mqttTopic; + this.requestRegex = new RegExp(`${settings.get().mqtt.base_topic}/bridge/request/${mqttTopic}/(save|remove)`); + this.basePath = data.joinPath(folderName); + } + + override async start(): Promise { + await super.start(); + this.eventBus.onMQTTMessage(this, this.onMQTTMessage); + await this.loadFiles(); + await this.publishExternalJS(); + } + + private getFilePath(name: string, mkBasePath: boolean = false): string { + if (mkBasePath && !fs.existsSync(this.basePath)) { + fs.mkdirSync(this.basePath, {recursive: true}); + } + + return path.join(this.basePath, name); + } + + protected getFileCode(name: string): string { + return fs.readFileSync(path.join(this.basePath, name), 'utf8'); + } + + protected *getFiles(): Generator<{name: string; code: string}> { + for (const fileName of fs.readdirSync(this.basePath)) { + if (fileName.endsWith('.js')) { + yield {name: fileName, code: this.getFileCode(fileName)}; + } + } + } + + @bind async onMQTTMessage(data: eventdata.MQTTMessage): Promise { + const match = data.topic.match(this.requestRegex); + + if (match && this.requestLookup[match[1].toLowerCase()]) { + const message = utils.parseJSON(data.message, data.message) as KeyValue; + + try { + const response = await this.requestLookup[match[1].toLowerCase()](message); + + await this.mqtt.publish(`bridge/response/${this.mqttTopic}/${match[1]}`, stringify(response)); + } catch (error) { + logger.error(`Request '${data.topic}' failed with error: '${(error as Error).message}'`); + + const response = utils.getResponse(message, {}, `${(error as Error).message}`); + + await this.mqtt.publish(`bridge/response/${this.mqttTopic}/${match[1]}`, stringify(response)); + } + } + } + + protected abstract removeJS(name: string, module: M): Promise; + + protected abstract loadJS(name: string, module: M): Promise; + + @bind private async remove(message: KeyValue): Promise { + const {name} = message; + const toBeRemoved = this.getFilePath(name); + + if (fs.existsSync(toBeRemoved)) { + await this.removeJS(name, this.loadModuleFromText(this.getFileCode(name), name)); + + 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`); + } + } + + @bind private async save(message: KeyValue): Promise { + const {name, code} = message; + + await this.loadJS(name, this.loadModuleFromText(code, name)); + + const filePath = this.getFilePath(name, true); + + fs.writeFileSync(filePath, code, 'utf8'); + logger.info(`${name} loaded. Contents written to '${filePath}'.`); + await this.publishExternalJS(); + + return utils.getResponse(message, {}); + } + + private async loadFiles(): Promise { + for (const extension of this.getFiles()) { + await this.loadJS(extension.name, this.loadModuleFromText(extension.code, extension.name)); + } + } + + private async publishExternalJS(): Promise { + await this.mqtt.publish( + `bridge/${this.mqttTopic}s`, + stringify(Array.from(this.getFiles())), + { + retain: true, + qos: 0, + }, + settings.get().mqtt.base_topic, + true, + ); + } + + private loadModuleFromText(moduleCode: string, name?: string): M { + const moduleFakePath = path.join(__dirname, '..', '..', 'data', 'extension', name || 'externally-loaded.js'); + const sandbox: Context = { + require: require, + module: {}, + console, + setTimeout, + clearTimeout, + setInterval, + clearInterval, + setImmediate, + clearImmediate, + }; + + runInNewContext(moduleCode, sandbox, moduleFakePath); + + return sandbox.module.exports; + } +} diff --git a/lib/util/utils.ts b/lib/util/utils.ts index 7985049725..d40eef7183 100644 --- a/lib/util/utils.ts +++ b/lib/util/utils.ts @@ -3,13 +3,10 @@ import type * as zhc from 'zigbee-herdsman-converters'; import assert from 'assert'; import fs from 'fs'; import path from 'path'; -import vm from 'vm'; import equals from 'fast-deep-equal/es6'; import humanizeDuration from 'humanize-duration'; -import data from './data'; - function pad(num: number): string { const norm = Math.floor(Math.abs(num)); return (norm < 10 ? '0' : '') + norm; @@ -145,48 +142,6 @@ function parseJSON(value: string, fallback: string): KeyValue | string { } } -function loadModuleFromText(moduleCode: string, name?: string): unknown { - const moduleFakePath = path.join(__dirname, '..', '..', 'data', 'extension', name || 'externally-loaded.js'); - const sandbox = { - require: require, - module: {}, - console, - setTimeout, - clearTimeout, - setInterval, - clearInterval, - setImmediate, - clearImmediate, - }; - vm.runInNewContext(moduleCode, sandbox, moduleFakePath); - /* eslint-disable-line */ // @ts-ignore - return sandbox.module.exports; -} - -function loadModuleFromFile(modulePath: string): unknown { - const moduleCode = fs.readFileSync(modulePath, {encoding: 'utf8'}); - return loadModuleFromText(moduleCode); -} - -export function* loadExternalConverter(moduleName: string): Generator { - let converter; - - if (moduleName.endsWith('.js')) { - converter = loadModuleFromFile(data.joinPath(moduleName)); - } else { - // eslint-disable-next-line @typescript-eslint/no-require-imports - converter = require(moduleName); - } - - if (Array.isArray(converter)) { - for (const item of converter) { - yield item; - } - } else { - yield converter; - } -} - /** * Delete all keys from passed object that have null/undefined values. * @@ -426,8 +381,6 @@ export default { getObjectProperty, getResponse, parseJSON, - loadModuleFromText, - loadModuleFromFile, removeNullPropertiesFromObject, toNetworkAddressHex, toSnakeCaseString, From 13e7da43b6e51e35b43c492203ee71b79ed9a78d Mon Sep 17 00:00:00 2001 From: Nerivec <62446222+Nerivec@users.noreply.github.com> Date: Thu, 14 Nov 2024 00:07:07 +0100 Subject: [PATCH 02/26] Fix imports --- lib/extension/externalJS.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/extension/externalJS.ts b/lib/extension/externalJS.ts index f496804a6a..073634cda6 100644 --- a/lib/extension/externalJS.ts +++ b/lib/extension/externalJS.ts @@ -4,11 +4,11 @@ import {Context, runInNewContext} from 'vm'; import bind from 'bind-decorator'; import stringify from 'json-stable-stringify-without-jsonify'; -import data from 'lib/util/data'; -import utils from 'lib/util/utils'; +import data from '../util/data'; +import logger from '../util/logger'; import * as settings from '../util/settings'; -import logger from './../util/logger'; +import utils from '../util/utils'; import Extension from './extension'; export default abstract class ExternalJSExtension extends Extension { From a0ba9868f35399ea00bf4944776ac110f529e06c Mon Sep 17 00:00:00 2001 From: Nerivec <62446222+Nerivec@users.noreply.github.com> Date: Thu, 14 Nov 2024 00:59:46 +0100 Subject: [PATCH 03/26] Improve error message on MQTT save --- lib/extension/externalJS.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/lib/extension/externalJS.ts b/lib/extension/externalJS.ts index 073634cda6..4cc951c0af 100644 --- a/lib/extension/externalJS.ts +++ b/lib/extension/externalJS.ts @@ -111,15 +111,19 @@ export default abstract class ExternalJSExtension extends Extension { @bind private async save(message: KeyValue): Promise { const {name, code} = message; - await this.loadJS(name, this.loadModuleFromText(code, name)); + try { + await this.loadJS(name, this.loadModuleFromText(code, name)); - const filePath = this.getFilePath(name, true); + const filePath = this.getFilePath(name, true); - fs.writeFileSync(filePath, code, 'utf8'); - logger.info(`${name} loaded. Contents written to '${filePath}'.`); - await this.publishExternalJS(); + fs.writeFileSync(filePath, code, 'utf8'); + logger.info(`${name} loaded. Contents written to '${filePath}'.`); + await this.publishExternalJS(); - return utils.getResponse(message, {}); + return utils.getResponse(message, {}); + } catch (error) { + return utils.getResponse(message, {}, `${name} contains invalid code: ${(error as Error).message}`); + } } private async loadFiles(): Promise { From 05d21d0d963fa7f65a5817a78d39a86232e87dc8 Mon Sep 17 00:00:00 2001 From: Nerivec <62446222+Nerivec@users.noreply.github.com> Date: Thu, 14 Nov 2024 01:19:23 +0100 Subject: [PATCH 04/26] Handle non-existing base path --- lib/extension/externalJS.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/extension/externalJS.ts b/lib/extension/externalJS.ts index 4cc951c0af..4b997269f3 100644 --- a/lib/extension/externalJS.ts +++ b/lib/extension/externalJS.ts @@ -60,6 +60,10 @@ export default abstract class ExternalJSExtension extends Extension { } protected *getFiles(): Generator<{name: string; code: string}> { + if (!fs.existsSync(this.basePath)) { + return; + } + for (const fileName of fs.readdirSync(this.basePath)) { if (fileName.endsWith('.js')) { yield {name: fileName, code: this.getFileCode(fileName)}; From b585a05614cbf138b7147925379e0567dac96a5b Mon Sep 17 00:00:00 2001 From: Nerivec <62446222+Nerivec@users.noreply.github.com> Date: Thu, 14 Nov 2024 01:43:53 +0100 Subject: [PATCH 05/26] Throw on bad converter --- lib/extension/externalConverters.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/extension/externalConverters.ts b/lib/extension/externalConverters.ts index 910b1cdf64..a958c3d33c 100644 --- a/lib/extension/externalConverters.ts +++ b/lib/extension/externalConverters.ts @@ -45,11 +45,13 @@ export default class ExternalConverters extends ExternalJSExtension Date: Thu, 14 Nov 2024 01:56:17 +0100 Subject: [PATCH 06/26] Add tests --- .../mock-external-converter-multiple.js | 0 .../mock-external-converter.js | 0 .../external_extensions/example2Extension.js | 16 + .../exampleExtension.js | 6 +- test/extensions/externalConverters.test.ts | 286 ++++++++++++------ test/extensions/externalExtension.test.ts | 149 --------- test/extensions/externalExtensions.test.ts | 209 +++++++++++++ 7 files changed, 417 insertions(+), 249 deletions(-) rename test/assets/{ => external_converters}/mock-external-converter-multiple.js (100%) rename test/assets/{ => external_converters}/mock-external-converter.js (100%) create mode 100644 test/assets/external_extensions/example2Extension.js rename test/assets/{ => external_extensions}/exampleExtension.js (61%) delete mode 100644 test/extensions/externalExtension.test.ts create mode 100644 test/extensions/externalExtensions.test.ts diff --git a/test/assets/mock-external-converter-multiple.js b/test/assets/external_converters/mock-external-converter-multiple.js similarity index 100% rename from test/assets/mock-external-converter-multiple.js rename to test/assets/external_converters/mock-external-converter-multiple.js diff --git a/test/assets/mock-external-converter.js b/test/assets/external_converters/mock-external-converter.js similarity index 100% rename from test/assets/mock-external-converter.js rename to test/assets/external_converters/mock-external-converter.js diff --git a/test/assets/external_extensions/example2Extension.js b/test/assets/external_extensions/example2Extension.js new file mode 100644 index 0000000000..258de01b71 --- /dev/null +++ b/test/assets/external_extensions/example2Extension.js @@ -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'); + } +} + +module.exports = Example2; diff --git a/test/assets/exampleExtension.js b/test/assets/external_extensions/exampleExtension.js similarity index 61% rename from test/assets/exampleExtension.js rename to test/assets/external_extensions/exampleExtension.js index 149cc96f48..aec02a8934 100644 --- a/test/assets/exampleExtension.js +++ b/test/assets/external_extensions/exampleExtension.js @@ -5,7 +5,11 @@ class Example { } start() { - this.mqtt.publish('example/extension', 'test'); + this.mqtt.publish('example/extension', 'call from start'); + } + + stop() { + this.mqtt.publish('example/extension', 'call from stop'); } } diff --git a/test/extensions/externalConverters.test.ts b/test/extensions/externalConverters.test.ts index e5ba4fb0b9..e84519e953 100644 --- a/test/extensions/externalConverters.test.ts +++ b/test/extensions/externalConverters.test.ts @@ -1,119 +1,109 @@ import * as data from '../mocks/data'; import {mockLogger} from '../mocks/logger'; -import {mockMQTT} from '../mocks/mqtt'; +import {mockMQTT, events as mockMQTTEvents} from '../mocks/mqtt'; import {flushPromises} from '../mocks/utils'; -import {devices, mockController as mockZHController} from '../mocks/zigbeeHerdsman'; +import {devices, mockController as mockZHController, returnDevices} from '../mocks/zigbeeHerdsman'; import fs from 'fs'; import path from 'path'; +import stringify from 'json-stable-stringify-without-jsonify'; + import * as zhc from 'zigbee-herdsman-converters'; import {Controller} from '../../lib/controller'; import * as settings from '../../lib/util/settings'; -jest.mock( - 'mock-external-converter-module', - () => { - return { - mock: true, - }; - }, - { - virtual: true, - }, -); - -jest.mock( - 'mock-multiple-external-converter-module', - () => { - return [ - { - mock: 1, - }, - { - mock: 2, - }, - ]; - }, - { - virtual: true, - }, -); +const BASE_DIR = 'external_converters'; const mockZHCAddDefinition = jest.fn(); +const mockZHCRemoveDefinition = jest.fn(); // @ts-expect-error mock zhc.addDefinition = mockZHCAddDefinition; - -const mocksClear = [ - mockZHCAddDefinition, - devices.bulb_color.removeFromNetwork, - devices.bulb.removeFromNetwork, - mockZHController.permitJoin, - mockZHController.stop, - mockMQTT.end, - mockMQTT.publish, - mockLogger.debug, - mockLogger.error, -]; +// @ts-expect-error mock +zhc.removeDefinition = mockZHCRemoveDefinition; describe('Extension: ExternalConverters', () => { + const mockBasePath = path.join(data.mockDir, BASE_DIR); let controller: Controller; - const resetExtension = async (): Promise => { - await controller.enableDisableExtension(false, 'ExternalConverters'); - await controller.enableDisableExtension(true, 'ExternalConverters'); + const existsSyncSpy = jest.spyOn(fs, 'existsSync'); + const readdirSyncSpy = jest.spyOn(fs, 'readdirSync'); + const mkdirSyncSpy = jest.spyOn(fs, 'mkdirSync'); + const rmSyncSpy = jest.spyOn(fs, 'rmSync'); + const writeFileSyncSpy = jest.spyOn(fs, 'writeFileSync'); + + const mocksClear = [ + mockMQTT.end, + mockMQTT.publish, + mockLogger.debug, + mockLogger.error, + mockZHController.stop, + devices.bulb.save, + mockZHCAddDefinition, + mockZHCRemoveDefinition, + existsSyncSpy, + readdirSyncSpy, + mkdirSyncSpy, + rmSyncSpy, + writeFileSyncSpy, + ]; + + const useAssets = (): void => { + fs.cpSync(path.join(__dirname, '..', 'assets', BASE_DIR), mockBasePath, {recursive: true}); + }; + + const getFileCode = (fileName: string): string => { + return fs.readFileSync(path.join(__dirname, '..', 'assets', BASE_DIR, fileName), 'utf8'); }; beforeAll(async () => { jest.useFakeTimers(); - controller = new Controller(jest.fn(), jest.fn()); - await controller.start(); - await flushPromises(); + }); + + afterAll(async () => { + jest.useRealTimers(); }); beforeEach(async () => { + mocksClear.forEach((m) => m.mockClear()); data.writeDefaultConfiguration(); + data.writeDefaultState(); settings.reRead(); - mocksClear.forEach((m) => m.mockClear()); - await resetExtension(); - }); + returnDevices.splice(0); - afterAll(async () => { - jest.useRealTimers(); + controller = new Controller(jest.fn(), jest.fn()); }); - it('Does not load external converters', async () => { - settings.set(['external_converters'], []); - await resetExtension(); - expect(mockZHCAddDefinition).toHaveBeenCalledTimes(0); + afterEach(() => { + fs.rmSync(mockBasePath, {recursive: true, force: true}); }); - it('Loads external converters', async () => { - fs.copyFileSync(path.join(__dirname, '..', 'assets', 'mock-external-converter.js'), path.join(data.mockDir, 'mock-external-converter.js')); - settings.set(['external_converters'], ['mock-external-converter.js']); - await resetExtension(); - expect(mockZHCAddDefinition).toHaveBeenCalledTimes(1); - expect(mockZHCAddDefinition).toHaveBeenCalledWith({ - mock: true, - zigbeeModel: ['external_converter_device'], - vendor: 'external', - model: 'external_converter_device', - description: 'external', - fromZigbee: [], - toZigbee: [], - exposes: [], - }); + it('loads nothing from folder', async () => { + await controller.start(); + await flushPromises(); + + expect(existsSyncSpy).toHaveBeenCalledWith(mockBasePath); + expect(readdirSyncSpy).not.toHaveBeenCalledWith(mockBasePath); + expect(mockMQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bridge/converters', stringify([]), {retain: true, qos: 0}, expect.any(Function)); }); - it('Loads multiple external converters', async () => { - fs.copyFileSync( - path.join(__dirname, '..', 'assets', 'mock-external-converter-multiple.js'), - path.join(data.mockDir, 'mock-external-converter-multiple.js'), + it('loads from folder', async () => { + useAssets(); + + await controller.start(); + await flushPromises(); + + expect(mockMQTT.publish).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')}, + ]), + {retain: true, qos: 0}, + expect.any(Function), ); - settings.set(['external_converters'], ['mock-external-converter-multiple.js']); - await resetExtension(); - expect(mockZHCAddDefinition).toHaveBeenCalledTimes(2); + expect(mockZHCRemoveDefinition).toHaveBeenCalledTimes(3); expect(mockZHCAddDefinition).toHaveBeenNthCalledWith(1, { mock: 1, model: 'external_converters_device_1', @@ -134,36 +124,134 @@ describe('Extension: ExternalConverters', () => { toZigbee: [], exposes: [], }); + expect(mockZHCAddDefinition).toHaveBeenNthCalledWith(3, { + mock: true, + zigbeeModel: ['external_converter_device'], + vendor: 'external', + model: 'external_converter_device', + description: 'external', + fromZigbee: [], + toZigbee: [], + exposes: [], + }); }); - it('Loads external converters from package', async () => { - settings.set(['external_converters'], ['mock-external-converter-module']); - await resetExtension(); - expect(mockZHCAddDefinition).toHaveBeenCalledTimes(1); + it('saves and removes from MQTT', async () => { + const converterName = 'foo.js'; + const converterCode = getFileCode('mock-external-converter.js'); + const converterFilePath = path.join(mockBasePath, converterName); + + await controller.start(); + await flushPromises(); + mocksClear.forEach((m) => m.mockClear()); + + //-- SAVE + mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/save', stringify({name: converterName, code: converterCode})); + await flushPromises(); + + expect(mkdirSyncSpy).toHaveBeenCalledWith(mockBasePath, {recursive: true}); + expect(writeFileSyncSpy).toHaveBeenCalledWith(converterFilePath, converterCode, 'utf8'); + expect(mockZHCRemoveDefinition).toHaveBeenCalledTimes(1); expect(mockZHCAddDefinition).toHaveBeenCalledWith({ mock: true, + zigbeeModel: ['external_converter_device'], + vendor: 'external', + model: 'external_converter_device', + description: 'external', + fromZigbee: [], + toZigbee: [], + exposes: [], }); - }); + expect(mockMQTT.publish).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/converters', + stringify([{name: converterName, code: converterCode}]), + {retain: true, qos: 0}, + expect.any(Function), + ); - it('Loads multiple external converters from package', async () => { - settings.set(['external_converters'], ['mock-multiple-external-converter-module']); - await resetExtension(); - expect(mockZHCAddDefinition).toHaveBeenCalledTimes(2); - expect(mockZHCAddDefinition).toHaveBeenNthCalledWith(1, { - mock: 1, - }); - expect(mockZHCAddDefinition).toHaveBeenNthCalledWith(2, { - mock: 2, + //-- REMOVE + mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/remove', stringify({name: converterName})); + await flushPromises(); + + expect(rmSyncSpy).toHaveBeenCalledWith(converterFilePath, {force: true}); + expect(mockZHCRemoveDefinition).toHaveBeenCalledWith({ + mock: true, + zigbeeModel: ['external_converter_device'], + vendor: 'external', + model: 'external_converter_device', + description: 'external', + fromZigbee: [], + toZigbee: [], + exposes: [], }); + expect(mockMQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bridge/converters', stringify([]), {retain: true, qos: 0}, expect.any(Function)); + }); + + 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 controller.start(); + await flushPromises(); + mocksClear.forEach((m) => m.mockClear()); + + mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/save', stringify({name: converterName, code: converterCode})); + await flushPromises(); + + expect(mockMQTT.publish).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/response/converter/save', + expect.stringContaining(`"error":"foo.js contains invalid code`), + {retain: false, qos: 0}, + expect.any(Function), + ); + expect(writeFileSyncSpy).not.toHaveBeenCalledWith(converterFilePath, converterCode, 'utf8'); }); - it('Loads external converters with error', async () => { - fs.copyFileSync(path.join(__dirname, '..', 'assets', 'mock-external-converter.js'), path.join(data.mockDir, 'mock-external-converter.js')); - settings.set(['external_converters'], ['mock-external-converter.js']); + it('returns error on invalid removal', async () => { + const converterName = 'invalid.js'; + const converterFilePath = path.join(mockBasePath, converterName); + + await controller.start(); + await flushPromises(); + mocksClear.forEach((m) => m.mockClear()); + + mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/remove', stringify({name: converterName})); + await flushPromises(); + + expect(mockMQTT.publish).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/response/converter/remove', + stringify({data: {}, status: 'error', error: `${converterName} (${converterFilePath}) doesn't exists`}), + {retain: false, qos: 0}, + expect.any(Function), + ); + expect(rmSyncSpy).not.toHaveBeenCalledWith(converterFilePath, {force: true}); + }); + + it('returns error on invalid definition', async () => { + const converterName = 'foo.js'; + const converterCode = getFileCode('mock-external-converter.js'); + const converterFilePath = path.join(mockBasePath, converterName); + + await controller.start(); + await flushPromises(); + mocksClear.forEach((m) => m.mockClear()); + + const errorMsg = `Invalid definition`; + mockZHCAddDefinition.mockImplementationOnce(() => { - throw new Error('Invalid definition!'); + throw new Error(errorMsg); }); - await resetExtension(); - expect(mockLogger.error).toHaveBeenCalledWith(`Failed to load external converter file 'mock-external-converter.js' (Invalid definition!)`); + + mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/save', stringify({name: converterName, code: converterCode})); + await flushPromises(); + + expect(mockMQTT.publish).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/response/converter/save', + expect.stringContaining(errorMsg), + {retain: false, qos: 0}, + expect.any(Function), + ); + expect(writeFileSyncSpy).not.toHaveBeenCalledWith(converterFilePath, converterCode, 'utf8'); }); }); diff --git a/test/extensions/externalExtension.test.ts b/test/extensions/externalExtension.test.ts deleted file mode 100644 index 20c3bbc6d9..0000000000 --- a/test/extensions/externalExtension.test.ts +++ /dev/null @@ -1,149 +0,0 @@ -import * as data from '../mocks/data'; -import {mockLogger} from '../mocks/logger'; -import {mockMQTT, events as mockMQTTEvents} from '../mocks/mqtt'; -import {flushPromises} from '../mocks/utils'; -import {devices, mockController as mockZHController, returnDevices} from '../mocks/zigbeeHerdsman'; - -import fs from 'fs'; -import path from 'path'; - -import stringify from 'json-stable-stringify-without-jsonify'; -import {rimrafSync} from 'rimraf'; - -import {Controller} from '../../lib/controller'; -import * as settings from '../../lib/util/settings'; - -const mocksClear = [ - mockZHController.permitJoin, - mockZHController.stop, - devices.bulb_color.removeFromNetwork, - devices.bulb.removeFromNetwork, - mockMQTT.end, - mockMQTT.publish, - mockLogger.debug, - mockLogger.error, -]; - -describe('Extension: ExternalExtension', () => { - let controller: Controller; - let mkdirSyncSpy: jest.SpyInstance; - let unlinkSyncSpy: jest.SpyInstance; - - beforeAll(async () => { - jest.useFakeTimers(); - mkdirSyncSpy = jest.spyOn(fs, 'mkdirSync'); - unlinkSyncSpy = jest.spyOn(fs, 'unlinkSync'); - }); - - afterAll(async () => { - jest.useRealTimers(); - }); - - beforeEach(async () => { - data.writeDefaultConfiguration(); - settings.reRead(); - mocksClear.forEach((m) => m.mockClear()); - returnDevices.splice(0); - mocksClear.forEach((m) => m.mockClear()); - data.writeDefaultConfiguration(); - settings.reRead(); - data.writeDefaultState(); - }); - - afterEach(() => { - const extensionPath = path.join(data.mockDir, 'extension'); - rimrafSync(extensionPath); - }); - - it('Load user extension', async () => { - const extensionPath = path.join(data.mockDir, 'extension'); - const extensionCode = fs.readFileSync(path.join(__dirname, '..', 'assets', 'exampleExtension.js'), 'utf-8'); - fs.mkdirSync(extensionPath); - fs.copyFileSync(path.join(__dirname, '..', 'assets', 'exampleExtension.js'), path.join(extensionPath, 'exampleExtension.js')); - controller = new Controller(jest.fn(), jest.fn()); - await controller.start(); - await flushPromises(); - expect(mockMQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/example/extension', 'test', {retain: false, qos: 0}, expect.any(Function)); - expect(mockMQTT.publish).toHaveBeenCalledWith( - 'zigbee2mqtt/bridge/extensions', - stringify([{name: 'exampleExtension.js', code: extensionCode}]), - {retain: true, qos: 0}, - expect.any(Function), - ); - }); - - it('Load user extension from api call', async () => { - const extensionPath = path.join(data.mockDir, 'extension'); - const extensionCode = fs.readFileSync(path.join(__dirname, '..', 'assets', 'exampleExtension.js'), 'utf-8'); - controller = new Controller(jest.fn(), jest.fn()); - await controller.start(); - await flushPromises(); - mockMQTT.publish.mockClear(); - mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/save', stringify({name: 'foo.js', code: extensionCode})); - await flushPromises(); - expect(mockMQTT.publish).toHaveBeenCalledWith( - 'zigbee2mqtt/bridge/extensions', - stringify([{name: 'foo.js', code: extensionCode}]), - {retain: true, qos: 0}, - expect.any(Function), - ); - expect(mockMQTT.publish).toHaveBeenCalledWith( - 'zigbee2mqtt/example/extension', - 'call from constructor', - {retain: false, qos: 0}, - expect.any(Function), - ); - expect(mkdirSyncSpy).toHaveBeenCalledWith(extensionPath); - }); - - it('Do not load corrupted extensions', async () => { - const extensionCode = 'definetly not a correct javascript code'; - controller = new Controller(jest.fn(), jest.fn()); - await controller.start(); - await flushPromises(); - mockMQTT.publish.mockClear(); - mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/save', stringify({name: 'foo.js', code: extensionCode})); - await flushPromises(); - - expect(mockMQTT.publish).toHaveBeenCalledWith( - 'zigbee2mqtt/bridge/response/extension/save', - expect.any(String), - {retain: false, qos: 0}, - expect.any(Function), - ); - const payload = JSON.parse(mockMQTT.publish.mock.calls[0][1]); - expect(payload).toEqual(expect.objectContaining({data: {}, status: 'error'})); - expect(payload.error).toMatch('Unexpected identifier'); - }); - - it('Removes user extension', async () => { - const extensionPath = path.join(data.mockDir, 'extension'); - const extensionCode = fs.readFileSync(path.join(__dirname, '..', 'assets', 'exampleExtension.js'), 'utf-8'); - fs.mkdirSync(extensionPath); - const extensionFilePath = path.join(extensionPath, 'exampleExtension.js'); - fs.copyFileSync(path.join(__dirname, '..', 'assets', 'exampleExtension.js'), extensionFilePath); - controller = new Controller(jest.fn(), jest.fn()); - await controller.start(); - await flushPromises(); - expect(mockMQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/example/extension', 'test', {retain: false, qos: 0}, expect.any(Function)); - expect(mockMQTT.publish).toHaveBeenCalledWith( - 'zigbee2mqtt/bridge/extensions', - stringify([{name: 'exampleExtension.js', code: extensionCode}]), - {retain: true, qos: 0}, - expect.any(Function), - ); - - mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/remove', stringify({name: 'exampleExtension.js'})); - await flushPromises(); - expect(unlinkSyncSpy).toHaveBeenCalledWith(extensionFilePath); - mockMQTT.publish.mockClear(); - mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/remove', stringify({name: 'non existing.js'})); - await flushPromises(); - expect(mockMQTT.publish).toHaveBeenCalledWith( - 'zigbee2mqtt/bridge/response/extension/remove', - stringify({data: {}, status: 'error', error: "Extension non existing.js doesn't exists"}), - {retain: false, qos: 0}, - expect.any(Function), - ); - }); -}); diff --git a/test/extensions/externalExtensions.test.ts b/test/extensions/externalExtensions.test.ts new file mode 100644 index 0000000000..ee5d654fd9 --- /dev/null +++ b/test/extensions/externalExtensions.test.ts @@ -0,0 +1,209 @@ +import * as data from '../mocks/data'; +import {mockLogger} from '../mocks/logger'; +import {mockMQTT, events as mockMQTTEvents} from '../mocks/mqtt'; +import {flushPromises} from '../mocks/utils'; +import {devices, mockController as mockZHController, returnDevices} from '../mocks/zigbeeHerdsman'; + +import fs from 'fs'; +import path from 'path'; + +import stringify from 'json-stable-stringify-without-jsonify'; + +import {Controller} from '../../lib/controller'; +import * as settings from '../../lib/util/settings'; + +const BASE_DIR = 'external_extensions'; + +describe('Extension: ExternalExtensions', () => { + let controller: Controller; + const mockBasePath = path.join(data.mockDir, BASE_DIR); + + const existsSyncSpy = jest.spyOn(fs, 'existsSync'); + const readdirSyncSpy = jest.spyOn(fs, 'readdirSync'); + const mkdirSyncSpy = jest.spyOn(fs, 'mkdirSync'); + const rmSyncSpy = jest.spyOn(fs, 'rmSync'); + const writeFileSyncSpy = jest.spyOn(fs, 'writeFileSync'); + + const mocksClear = [ + mockMQTT.end, + mockMQTT.publish, + mockLogger.debug, + mockLogger.error, + mockZHController.stop, + devices.bulb.save, + existsSyncSpy, + readdirSyncSpy, + mkdirSyncSpy, + rmSyncSpy, + writeFileSyncSpy, + ]; + + const useAssets = (): void => { + fs.cpSync(path.join(__dirname, '..', 'assets', BASE_DIR), mockBasePath, {recursive: true}); + }; + + const getFileCode = (fileName: string): string => { + return fs.readFileSync(path.join(__dirname, '..', 'assets', BASE_DIR, fileName), 'utf8'); + }; + + beforeAll(async () => { + jest.useFakeTimers(); + }); + + afterAll(async () => { + jest.useRealTimers(); + }); + + beforeEach(async () => { + mocksClear.forEach((m) => m.mockClear()); + data.writeDefaultConfiguration(); + data.writeDefaultState(); + settings.reRead(); + returnDevices.splice(0); + + controller = new Controller(jest.fn(), jest.fn()); + }); + + afterEach(() => { + fs.rmSync(mockBasePath, {recursive: true, force: true}); + }); + + it('loads nothing from folder', async () => { + await controller.start(); + await flushPromises(); + + expect(existsSyncSpy).toHaveBeenCalledWith(mockBasePath); + expect(readdirSyncSpy).not.toHaveBeenCalledWith(mockBasePath); + expect(mockMQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bridge/extensions', stringify([]), {retain: true, qos: 0}, expect.any(Function)); + }); + + it('loads from folder', async () => { + useAssets(); + + await controller.start(); + await flushPromises(); + + expect(mockMQTT.publish).toHaveBeenCalledWith( + 'zigbee2mqtt/example2/extension', + 'call2 from constructor', + {retain: false, qos: 0}, + expect.any(Function), + ); + expect(mockMQTT.publish).toHaveBeenCalledWith( + 'zigbee2mqtt/example2/extension', + 'call2 from start', + {retain: false, qos: 0}, + expect.any(Function), + ); + expect(mockMQTT.publish).toHaveBeenCalledWith( + 'zigbee2mqtt/example/extension', + 'call from constructor', + {retain: false, qos: 0}, + expect.any(Function), + ); + expect(mockMQTT.publish).toHaveBeenCalledWith( + 'zigbee2mqtt/example/extension', + 'call from start', + {retain: false, qos: 0}, + expect.any(Function), + ); + expect(mockMQTT.publish).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/extensions', + stringify([ + {name: 'example2Extension.js', code: getFileCode('example2Extension.js')}, + {name: 'exampleExtension.js', code: getFileCode('exampleExtension.js')}, + ]), + {retain: true, qos: 0}, + expect.any(Function), + ); + }); + + it('saves and removes from MQTT', async () => { + const extensionName = 'foo.js'; + const extensionCode = getFileCode('exampleExtension.js'); + const extensionFilePath = path.join(mockBasePath, extensionName); + + await controller.start(); + await flushPromises(); + mocksClear.forEach((m) => m.mockClear()); + + //-- SAVE + mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/save', stringify({name: extensionName, code: extensionCode})); + await flushPromises(); + + expect(mkdirSyncSpy).toHaveBeenCalledWith(mockBasePath, {recursive: true}); + expect(writeFileSyncSpy).toHaveBeenCalledWith(extensionFilePath, extensionCode, 'utf8'); + expect(mockMQTT.publish).toHaveBeenCalledWith( + 'zigbee2mqtt/example/extension', + 'call from constructor', + {retain: false, qos: 0}, + expect.any(Function), + ); + expect(mockMQTT.publish).toHaveBeenCalledWith( + 'zigbee2mqtt/example/extension', + 'call from start', + {retain: false, qos: 0}, + expect.any(Function), + ); + expect(mockMQTT.publish).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/extensions', + stringify([{name: extensionName, code: extensionCode}]), + {retain: true, qos: 0}, + expect.any(Function), + ); + + //-- REMOVE + mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/remove', stringify({name: extensionName})); + await flushPromises(); + + expect(rmSyncSpy).toHaveBeenCalledWith(extensionFilePath, {force: true}); + expect(mockMQTT.publish).toHaveBeenCalledWith( + 'zigbee2mqtt/example/extension', + 'call from stop', + {retain: false, qos: 0}, + expect.any(Function), + ); + expect(mockMQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bridge/extensions', stringify([]), {retain: true, qos: 0}, expect.any(Function)); + }); + + 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); + + await controller.start(); + await flushPromises(); + mocksClear.forEach((m) => m.mockClear()); + + mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/save', stringify({name: extensionName, code: extensionCode})); + await flushPromises(); + + expect(mockMQTT.publish).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/response/extension/save', + expect.stringContaining(`"error":"${extensionName} contains invalid code`), + {retain: false, qos: 0}, + expect.any(Function), + ); + expect(writeFileSyncSpy).not.toHaveBeenCalledWith(extensionFilePath, extensionCode, 'utf8'); + }); + + it('returns error on invalid removal', async () => { + const converterName = 'invalid.js'; + const converterFilePath = path.join(mockBasePath, converterName); + + await controller.start(); + await flushPromises(); + mocksClear.forEach((m) => m.mockClear()); + + mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/remove', stringify({name: converterName})); + await flushPromises(); + + expect(mockMQTT.publish).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/response/extension/remove', + stringify({data: {}, status: 'error', error: `${converterName} (${converterFilePath}) doesn't exists`}), + {retain: false, qos: 0}, + expect.any(Function), + ); + expect(rmSyncSpy).not.toHaveBeenCalledWith(converterFilePath, {force: true}); + }); +}); From 2d4954713a83738ca33b945c78a5f0cb89316cf7 Mon Sep 17 00:00:00 2001 From: Nerivec <62446222+Nerivec@users.noreply.github.com> Date: Thu, 14 Nov 2024 13:23:21 +0100 Subject: [PATCH 07/26] Fix use of ext conv in network map tests. --- test/extensions/networkMap.test.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/extensions/networkMap.test.ts b/test/extensions/networkMap.test.ts index f8f347de97..002dabcdad 100644 --- a/test/extensions/networkMap.test.ts +++ b/test/extensions/networkMap.test.ts @@ -102,8 +102,11 @@ describe('Extension: NetworkMap', () => { data.writeDefaultConfiguration(); settings.reRead(); data.writeEmptyState(); - fs.copyFileSync(path.join(__dirname, '..', 'assets', 'mock-external-converter.js'), path.join(data.mockDir, 'mock-external-converter.js')); - settings.set(['external_converters'], ['mock-external-converter.js']); + fs.mkdirSync(path.join(data.mockDir, 'external_converters')); + fs.copyFileSync( + path.join(__dirname, '..', 'assets', 'external_converters', 'mock-external-converter.js'), + path.join(data.mockDir, 'external_converters', 'mock-external-converter.js'), + ); controller = new Controller(jest.fn(), jest.fn()); await controller.start(); }); @@ -120,6 +123,7 @@ describe('Extension: NetworkMap', () => { afterAll(async () => { mockSleep.restore(); + fs.rmSync(path.join(data.mockDir, 'external_converters'), {recursive: true}); jest.useRealTimers(); }); From 5a2c69fdcb21ea41b1fedd2f63550e1a927aa437 Mon Sep 17 00:00:00 2001 From: Nerivec <62446222+Nerivec@users.noreply.github.com> Date: Thu, 14 Nov 2024 13:28:46 +0100 Subject: [PATCH 08/26] More coverage. --- lib/extension/externalJS.ts | 5 ++-- test/extensions/externalConverters.test.ts | 32 ++++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/lib/extension/externalJS.ts b/lib/extension/externalJS.ts index 4b997269f3..599adc8a06 100644 --- a/lib/extension/externalJS.ts +++ b/lib/extension/externalJS.ts @@ -65,6 +65,7 @@ export default abstract class ExternalJSExtension extends Extension { } for (const fileName of fs.readdirSync(this.basePath)) { + /* istanbul ignore else */ if (fileName.endsWith('.js')) { yield {name: fileName, code: this.getFileCode(fileName)}; } @@ -149,8 +150,8 @@ export default abstract class ExternalJSExtension extends Extension { ); } - private loadModuleFromText(moduleCode: string, name?: string): M { - const moduleFakePath = path.join(__dirname, '..', '..', 'data', 'extension', name || 'externally-loaded.js'); + private loadModuleFromText(moduleCode: string, name: string): M { + const moduleFakePath = path.join(__dirname, '..', '..', 'data', 'extension', name); const sandbox: Context = { require: require, module: {}, diff --git a/test/extensions/externalConverters.test.ts b/test/extensions/externalConverters.test.ts index e84519e953..677593e60d 100644 --- a/test/extensions/externalConverters.test.ts +++ b/test/extensions/externalConverters.test.ts @@ -254,4 +254,36 @@ describe('Extension: ExternalConverters', () => { ); 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); + + await controller.start(); + await flushPromises(); + mocksClear.forEach((m) => m.mockClear()); + + //-- SAVE + mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/save', stringify({name: converterName, code: converterCode})); + await flushPromises(); + + const errorMsg = `Failed to remove definition`; + + mockZHCRemoveDefinition.mockImplementationOnce(() => { + throw new Error(errorMsg); + }); + + //-- REMOVE + mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/remove', stringify({name: converterName})); + await flushPromises(); + + expect(mockMQTT.publish).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/response/converter/remove', + stringify({data: {}, status: 'error', error: errorMsg}), + {retain: false, qos: 0}, + expect.any(Function), + ); + expect(rmSyncSpy).not.toHaveBeenCalledWith(converterFilePath, {force: true}); + }); }); From 57aac29f7ec90df3088d020f7028dcbea73200e5 Mon Sep 17 00:00:00 2001 From: Nerivec <62446222+Nerivec@users.noreply.github.com> Date: Thu, 14 Nov 2024 14:17:26 +0100 Subject: [PATCH 09/26] Dont mock zhc for basics, tests actual live loading --- test/extensions/externalConverters.test.ts | 153 ++++++++++++--------- 1 file changed, 91 insertions(+), 62 deletions(-) diff --git a/test/extensions/externalConverters.test.ts b/test/extensions/externalConverters.test.ts index 677593e60d..7cd90dad1a 100644 --- a/test/extensions/externalConverters.test.ts +++ b/test/extensions/externalConverters.test.ts @@ -4,6 +4,8 @@ import {mockMQTT, events as mockMQTTEvents} from '../mocks/mqtt'; import {flushPromises} from '../mocks/utils'; import {devices, mockController as mockZHController, returnDevices} from '../mocks/zigbeeHerdsman'; +import type Device from '../../lib/model/device'; + import fs from 'fs'; import path from 'path'; @@ -16,12 +18,8 @@ import * as settings from '../../lib/util/settings'; const BASE_DIR = 'external_converters'; -const mockZHCAddDefinition = jest.fn(); -const mockZHCRemoveDefinition = jest.fn(); -// @ts-expect-error mock -zhc.addDefinition = mockZHCAddDefinition; -// @ts-expect-error mock -zhc.removeDefinition = mockZHCRemoveDefinition; +// @ts-expect-error TODO: remove, tmp until implemented +zhc.removeDefinition = jest.fn(); describe('Extension: ExternalConverters', () => { const mockBasePath = path.join(data.mockDir, BASE_DIR); @@ -33,6 +31,9 @@ describe('Extension: ExternalConverters', () => { const rmSyncSpy = jest.spyOn(fs, 'rmSync'); const writeFileSyncSpy = jest.spyOn(fs, 'writeFileSync'); + const zhcAddDefinitionSpy = jest.spyOn(zhc, 'addDefinition'); + const zhcRemoveDefinitionSpy = jest.spyOn(zhc, 'removeDefinition'); + const mocksClear = [ mockMQTT.end, mockMQTT.publish, @@ -40,13 +41,13 @@ describe('Extension: ExternalConverters', () => { mockLogger.error, mockZHController.stop, devices.bulb.save, - mockZHCAddDefinition, - mockZHCRemoveDefinition, existsSyncSpy, readdirSyncSpy, mkdirSyncSpy, rmSyncSpy, writeFileSyncSpy, + zhcAddDefinitionSpy, + zhcRemoveDefinitionSpy, ]; const useAssets = (): void => { @@ -57,6 +58,11 @@ describe('Extension: ExternalConverters', () => { return fs.readFileSync(path.join(__dirname, '..', 'assets', BASE_DIR, fileName), 'utf8'); }; + const getZ2MDevice = (zhDevice: unknown): Device => { + // @ts-expect-error private + return controller.zigbee.resolveEntity(zhDevice)! as Device; + }; + beforeAll(async () => { jest.useFakeTimers(); }); @@ -70,7 +76,7 @@ describe('Extension: ExternalConverters', () => { data.writeDefaultConfiguration(); data.writeDefaultState(); settings.reRead(); - returnDevices.splice(0); + returnDevices.push(devices.external_converter_device.ieeeAddr); controller = new Controller(jest.fn(), jest.fn()); }); @@ -94,6 +100,12 @@ describe('Extension: ExternalConverters', () => { await controller.start(); await flushPromises(); + expect(getZ2MDevice(devices.external_converter_device)!.definition).toMatchObject({ + description: 'external', + model: 'external_converter_device', + vendor: 'external', + zigbeeModel: ['external_converter_device'], + }); expect(mockMQTT.publish).toHaveBeenCalledWith( 'zigbee2mqtt/bridge/converters', stringify([ @@ -103,37 +115,37 @@ describe('Extension: ExternalConverters', () => { {retain: true, qos: 0}, expect.any(Function), ); - expect(mockZHCRemoveDefinition).toHaveBeenCalledTimes(3); - expect(mockZHCAddDefinition).toHaveBeenNthCalledWith(1, { - mock: 1, - model: 'external_converters_device_1', - zigbeeModel: ['external_converter_device_1'], - vendor: 'external_1', - description: 'external_1', - fromZigbee: [], - toZigbee: [], - exposes: [], - }); - expect(mockZHCAddDefinition).toHaveBeenNthCalledWith(2, { - mock: 2, - model: 'external_converters_device_2', - zigbeeModel: ['external_converter_device_2'], - vendor: 'external_2', - description: 'external_2', - fromZigbee: [], - toZigbee: [], - exposes: [], - }); - expect(mockZHCAddDefinition).toHaveBeenNthCalledWith(3, { - mock: true, - zigbeeModel: ['external_converter_device'], - vendor: 'external', - model: 'external_converter_device', - description: 'external', - fromZigbee: [], - toZigbee: [], - exposes: [], - }); + expect(zhcRemoveDefinitionSpy).toHaveBeenCalledTimes(3); + expect(zhcAddDefinitionSpy).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + mock: 1, + model: 'external_converters_device_1', + zigbeeModel: ['external_converter_device_1'], + vendor: 'external_1', + description: 'external_1', + }), + ); + expect(zhcAddDefinitionSpy).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + mock: 2, + model: 'external_converters_device_2', + zigbeeModel: ['external_converter_device_2'], + vendor: 'external_2', + description: 'external_2', + }), + ); + expect(zhcAddDefinitionSpy).toHaveBeenNthCalledWith( + 3, + expect.objectContaining({ + mock: true, + zigbeeModel: ['external_converter_device'], + vendor: 'external', + model: 'external_converter_device', + description: 'external', + }), + ); }); it('saves and removes from MQTT', async () => { @@ -145,23 +157,35 @@ describe('Extension: ExternalConverters', () => { await flushPromises(); mocksClear.forEach((m) => m.mockClear()); + expect(getZ2MDevice(devices.external_converter_device)!.definition).toMatchObject({ + description: 'Automatically generated definition', + model: 'external_converter_device', + vendor: '', + zigbeeModel: ['external_converter_device'], + }); + //-- SAVE mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/save', stringify({name: converterName, code: converterCode})); await flushPromises(); - expect(mkdirSyncSpy).toHaveBeenCalledWith(mockBasePath, {recursive: true}); - expect(writeFileSyncSpy).toHaveBeenCalledWith(converterFilePath, converterCode, 'utf8'); - expect(mockZHCRemoveDefinition).toHaveBeenCalledTimes(1); - expect(mockZHCAddDefinition).toHaveBeenCalledWith({ - mock: true, - zigbeeModel: ['external_converter_device'], - vendor: 'external', - model: 'external_converter_device', + expect(getZ2MDevice(devices.external_converter_device)!.definition).toMatchObject({ description: 'external', - fromZigbee: [], - toZigbee: [], - exposes: [], + model: 'external_converter_device', + vendor: 'external', + zigbeeModel: ['external_converter_device'], }); + expect(mkdirSyncSpy).toHaveBeenCalledWith(mockBasePath, {recursive: true}); + expect(writeFileSyncSpy).toHaveBeenCalledWith(converterFilePath, converterCode, 'utf8'); + expect(zhcRemoveDefinitionSpy).toHaveBeenCalledTimes(1); + expect(zhcAddDefinitionSpy).toHaveBeenCalledWith( + expect.objectContaining({ + mock: true, + zigbeeModel: ['external_converter_device'], + vendor: 'external', + model: 'external_converter_device', + description: 'external', + }), + ); expect(mockMQTT.publish).toHaveBeenCalledWith( 'zigbee2mqtt/bridge/converters', stringify([{name: converterName, code: converterCode}]), @@ -173,17 +197,22 @@ describe('Extension: ExternalConverters', () => { mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/remove', stringify({name: converterName})); await flushPromises(); - expect(rmSyncSpy).toHaveBeenCalledWith(converterFilePath, {force: true}); - expect(mockZHCRemoveDefinition).toHaveBeenCalledWith({ - mock: true, - zigbeeModel: ['external_converter_device'], - vendor: 'external', + expect(getZ2MDevice(devices.external_converter_device)!.definition).toMatchObject({ + description: 'Automatically generated definition', model: 'external_converter_device', - description: 'external', - fromZigbee: [], - toZigbee: [], - exposes: [], + vendor: '', + zigbeeModel: ['external_converter_device'], }); + expect(rmSyncSpy).toHaveBeenCalledWith(converterFilePath, {force: true}); + expect(zhcRemoveDefinitionSpy).toHaveBeenCalledWith( + expect.objectContaining({ + mock: true, + zigbeeModel: ['external_converter_device'], + vendor: 'external', + model: 'external_converter_device', + description: 'external', + }), + ); expect(mockMQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bridge/converters', stringify([]), {retain: true, qos: 0}, expect.any(Function)); }); @@ -239,7 +268,7 @@ describe('Extension: ExternalConverters', () => { const errorMsg = `Invalid definition`; - mockZHCAddDefinition.mockImplementationOnce(() => { + zhcAddDefinitionSpy.mockImplementationOnce(() => { throw new Error(errorMsg); }); @@ -270,7 +299,7 @@ describe('Extension: ExternalConverters', () => { const errorMsg = `Failed to remove definition`; - mockZHCRemoveDefinition.mockImplementationOnce(() => { + zhcRemoveDefinitionSpy.mockImplementationOnce(() => { throw new Error(errorMsg); }); From 0d5a8fd15452773ce093d14de3247762a03d2161 Mon Sep 17 00:00:00 2001 From: Nerivec <62446222+Nerivec@users.noreply.github.com> Date: Sun, 17 Nov 2024 20:22:35 +0100 Subject: [PATCH 10/26] Update --- lib/extension/externalConverters.ts | 38 +++++++++++--------- lib/model/device.ts | 2 +- lib/zigbee.ts | 10 ++++-- test/extensions/externalConverters.test.ts | 40 ++++++++++------------ 4 files changed, 47 insertions(+), 43 deletions(-) diff --git a/lib/extension/externalConverters.ts b/lib/extension/externalConverters.ts index a958c3d33c..79eefe0f69 100644 --- a/lib/extension/externalConverters.ts +++ b/lib/extension/externalConverters.ts @@ -1,4 +1,4 @@ -import {addDefinition, removeDefinition} from 'zigbee-herdsman-converters'; +import {addDefinition, removeExternalDefinitions} from 'zigbee-herdsman-converters'; import logger from '../util/logger'; import ExternalJSExtension from './externalJS'; @@ -30,29 +30,33 @@ export default class ExternalConverters extends ExternalJSExtension { - for (const definition of this.getDefinitions(module)) { - // TODO: implement in ZHC - removeDefinition(definition); - } + removeExternalDefinitions(name); + + await this.zigbee.resolveDevicesDefinitions(true); } protected async loadJS(name: string, module: ModuleExports): Promise { - for (const definition of this.getDefinitions(module)) { - try { - // TODO: `updateDefinition` in ZHC instead? (add if not exist, replace if exist) - removeDefinition(definition); + try { + removeExternalDefinitions(name); + + for (const definition of this.getDefinitions(module)) { + definition.externalConverterName = name; + addDefinition(definition); logger.info(`Loaded external converter '${name}'.`); - } catch (error) { - logger.error(`Failed to load external converter '${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`, - ); - - throw error; } + + await this.zigbee.resolveDevicesDefinitions(true); + } catch (error) { + logger.error(`Failed to load external converter '${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`, + ); + + throw error; } } diff --git a/lib/model/device.ts b/lib/model/device.ts index 0032eea5b6..51b45728d8 100644 --- a/lib/model/device.ts +++ b/lib/model/device.ts @@ -48,7 +48,7 @@ export default class Device { } } - async resolveDefinition(ignoreCache = false): Promise { + async resolveDefinition(ignoreCache: boolean = false): Promise { if (!this.zh.interviewing && (!this.definition || this._definitionModelID !== this.zh.modelID || ignoreCache)) { this.definition = await zhc.findByDevice(this.zh, true); this._definitionModelID = this.zh.modelID; diff --git a/lib/zigbee.ts b/lib/zigbee.ts index 8a7e5e2b52..18a173ac3a 100644 --- a/lib/zigbee.ts +++ b/lib/zigbee.ts @@ -72,9 +72,7 @@ export default class Zigbee { throw error; } - for (const device of this.devicesIterator(utils.deviceNotCoordinator)) { - await device.resolveDefinition(); - } + await this.resolveDevicesDefinitions(); this.herdsman.on('adapterDisconnected', () => this.eventBus.emitAdapterDisconnected()); this.herdsman.on('lastSeenChanged', (data: ZHEvents.LastSeenChangedPayload) => { @@ -239,6 +237,12 @@ export default class Zigbee { await this.herdsman.permitJoin(time, device?.zh); } + async resolveDevicesDefinitions(ignoreCache: boolean = false): Promise { + for (const device of this.devicesIterator(utils.deviceNotCoordinator)) { + await device.resolveDefinition(ignoreCache); + } + } + @bind private resolveDevice(ieeeAddr: string): Device | undefined { if (!this.deviceLookup[ieeeAddr]) { const device = this.herdsman.getDeviceByIeeeAddr(ieeeAddr); diff --git a/test/extensions/externalConverters.test.ts b/test/extensions/externalConverters.test.ts index 7cd90dad1a..b1eb375582 100644 --- a/test/extensions/externalConverters.test.ts +++ b/test/extensions/externalConverters.test.ts @@ -18,9 +18,6 @@ import * as settings from '../../lib/util/settings'; const BASE_DIR = 'external_converters'; -// @ts-expect-error TODO: remove, tmp until implemented -zhc.removeDefinition = jest.fn(); - describe('Extension: ExternalConverters', () => { const mockBasePath = path.join(data.mockDir, BASE_DIR); let controller: Controller; @@ -32,7 +29,7 @@ describe('Extension: ExternalConverters', () => { const writeFileSyncSpy = jest.spyOn(fs, 'writeFileSync'); const zhcAddDefinitionSpy = jest.spyOn(zhc, 'addDefinition'); - const zhcRemoveDefinitionSpy = jest.spyOn(zhc, 'removeDefinition'); + const zhcRemoveExternalDefinitionsSpy = jest.spyOn(zhc, 'removeExternalDefinitions'); const mocksClear = [ mockMQTT.end, @@ -47,7 +44,7 @@ describe('Extension: ExternalConverters', () => { rmSyncSpy, writeFileSyncSpy, zhcAddDefinitionSpy, - zhcRemoveDefinitionSpy, + zhcRemoveExternalDefinitionsSpy, ]; const useAssets = (): void => { @@ -72,6 +69,7 @@ describe('Extension: ExternalConverters', () => { }); beforeEach(async () => { + zhc.removeExternalDefinitions(); // remove all external converters mocksClear.forEach((m) => m.mockClear()); data.writeDefaultConfiguration(); data.writeDefaultState(); @@ -81,8 +79,10 @@ describe('Extension: ExternalConverters', () => { controller = new Controller(jest.fn(), jest.fn()); }); - afterEach(() => { + afterEach(async () => { fs.rmSync(mockBasePath, {recursive: true, force: true}); + + await controller?.stop(); }); it('loads nothing from folder', async () => { @@ -100,7 +100,7 @@ describe('Extension: ExternalConverters', () => { await controller.start(); await flushPromises(); - expect(getZ2MDevice(devices.external_converter_device)!.definition).toMatchObject({ + expect(getZ2MDevice(devices.external_converter_device).definition).toMatchObject({ description: 'external', model: 'external_converter_device', vendor: 'external', @@ -115,7 +115,9 @@ describe('Extension: ExternalConverters', () => { {retain: true, qos: 0}, expect.any(Function), ); - expect(zhcRemoveDefinitionSpy).toHaveBeenCalledTimes(3); + expect(zhcRemoveExternalDefinitionsSpy).toHaveBeenCalledTimes(2); + expect(zhcRemoveExternalDefinitionsSpy).toHaveBeenNthCalledWith(1, 'mock-external-converter-multiple.js'); + expect(zhcRemoveExternalDefinitionsSpy).toHaveBeenNthCalledWith(2, 'mock-external-converter.js'); expect(zhcAddDefinitionSpy).toHaveBeenNthCalledWith( 1, expect.objectContaining({ @@ -157,7 +159,7 @@ describe('Extension: ExternalConverters', () => { await flushPromises(); mocksClear.forEach((m) => m.mockClear()); - expect(getZ2MDevice(devices.external_converter_device)!.definition).toMatchObject({ + expect(getZ2MDevice(devices.external_converter_device).definition).toMatchObject({ description: 'Automatically generated definition', model: 'external_converter_device', vendor: '', @@ -168,7 +170,7 @@ describe('Extension: ExternalConverters', () => { mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/save', stringify({name: converterName, code: converterCode})); await flushPromises(); - expect(getZ2MDevice(devices.external_converter_device)!.definition).toMatchObject({ + expect(getZ2MDevice(devices.external_converter_device).definition).toMatchObject({ description: 'external', model: 'external_converter_device', vendor: 'external', @@ -176,7 +178,8 @@ describe('Extension: ExternalConverters', () => { }); expect(mkdirSyncSpy).toHaveBeenCalledWith(mockBasePath, {recursive: true}); expect(writeFileSyncSpy).toHaveBeenCalledWith(converterFilePath, converterCode, 'utf8'); - expect(zhcRemoveDefinitionSpy).toHaveBeenCalledTimes(1); + expect(zhcRemoveExternalDefinitionsSpy).toHaveBeenCalledTimes(1); + expect(zhcRemoveExternalDefinitionsSpy).toHaveBeenNthCalledWith(1, converterName); expect(zhcAddDefinitionSpy).toHaveBeenCalledWith( expect.objectContaining({ mock: true, @@ -197,22 +200,15 @@ describe('Extension: ExternalConverters', () => { mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/remove', stringify({name: converterName})); await flushPromises(); - expect(getZ2MDevice(devices.external_converter_device)!.definition).toMatchObject({ + 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(zhcRemoveDefinitionSpy).toHaveBeenCalledWith( - expect.objectContaining({ - mock: true, - zigbeeModel: ['external_converter_device'], - vendor: 'external', - model: 'external_converter_device', - description: 'external', - }), - ); + expect(zhcRemoveExternalDefinitionsSpy).toHaveBeenCalledTimes(2); + expect(zhcRemoveExternalDefinitionsSpy).toHaveBeenNthCalledWith(2, converterName); expect(mockMQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bridge/converters', stringify([]), {retain: true, qos: 0}, expect.any(Function)); }); @@ -299,7 +295,7 @@ describe('Extension: ExternalConverters', () => { const errorMsg = `Failed to remove definition`; - zhcRemoveDefinitionSpy.mockImplementationOnce(() => { + zhcRemoveExternalDefinitionsSpy.mockImplementationOnce(() => { throw new Error(errorMsg); }); From 09142617a31e6574a992cc7f999a7d5788456058 Mon Sep 17 00:00:00 2001 From: Nerivec <62446222+Nerivec@users.noreply.github.com> Date: Wed, 13 Nov 2024 00:26:32 +0100 Subject: [PATCH 11/26] feat: Live loading of external JS (converters/extensions) --- lib/controller.ts | 11 +- lib/extension/externalConverters.ts | 56 ++++++---- lib/extension/externalExtension.ts | 120 --------------------- lib/extension/externalExtensions.ts | 59 ++++++++++ lib/extension/externalJS.ts | 162 ++++++++++++++++++++++++++++ lib/util/utils.ts | 47 -------- 6 files changed, 263 insertions(+), 192 deletions(-) delete mode 100644 lib/extension/externalExtension.ts create mode 100644 lib/extension/externalExtensions.ts create mode 100644 lib/extension/externalJS.ts diff --git a/lib/controller.ts b/lib/controller.ts index 94b3037b49..3a93d4555a 100644 --- a/lib/controller.ts +++ b/lib/controller.ts @@ -15,7 +15,7 @@ import ExtensionBind from './extension/bind'; import ExtensionBridge from './extension/bridge'; import ExtensionConfigure from './extension/configure'; import ExtensionExternalConverters from './extension/externalConverters'; -import ExtensionExternalExtension from './extension/externalExtension'; +import ExtensionExternalExtensions from './extension/externalExtensions'; // Extensions import ExtensionFrontend from './extension/frontend'; import ExtensionGroups from './extension/groups'; @@ -47,7 +47,7 @@ const AllExtensions = [ ExtensionOTAUpdate, ExtensionExternalConverters, ExtensionFrontend, - ExtensionExternalExtension, + ExtensionExternalExtensions, ExtensionAvailability, ]; @@ -106,7 +106,8 @@ export class Controller { new ExtensionGroups(...this.extensionArgs), new ExtensionBind(...this.extensionArgs), new ExtensionOTAUpdate(...this.extensionArgs), - new ExtensionExternalExtension(...this.extensionArgs), + new ExtensionExternalExtensions(...this.extensionArgs), + new ExtensionExternalConverters(...this.extensionArgs), new ExtensionAvailability(...this.extensionArgs), ]; @@ -114,10 +115,6 @@ export class Controller { this.extensions.push(new ExtensionFrontend(...this.extensionArgs)); } - if (settings.get().external_converters.length) { - this.extensions.push(new ExtensionExternalConverters(...this.extensionArgs)); - } - if (settings.get().homeassistant) { this.extensions.push(new ExtensionHomeAssistant(...this.extensionArgs)); } diff --git a/lib/extension/externalConverters.ts b/lib/extension/externalConverters.ts index 06d4e77892..910b1cdf64 100644 --- a/lib/extension/externalConverters.ts +++ b/lib/extension/externalConverters.ts @@ -1,11 +1,11 @@ -import * as zhc from 'zigbee-herdsman-converters'; +import {addDefinition, removeDefinition} from 'zigbee-herdsman-converters'; import logger from '../util/logger'; -import * as settings from '../util/settings'; -import {loadExternalConverter} from '../util/utils'; -import Extension from './extension'; +import ExternalJSExtension from './externalJS'; -export default class ExternalConverters extends Extension { +type ModuleExports = ExternalDefinition | ExternalDefinition[]; + +export default class ExternalConverters extends ExternalJSExtension { constructor( zigbee: Zigbee, mqtt: MQTT, @@ -16,25 +16,45 @@ export default class ExternalConverters extends Extension { restartCallback: () => Promise, addExtension: (extension: Extension) => Promise, ) { - super(zigbee, mqtt, state, publishEntityState, eventBus, enableDisableExtension, restartCallback, addExtension); + super( + zigbee, + mqtt, + state, + publishEntityState, + eventBus, + enableDisableExtension, + restartCallback, + addExtension, + 'converter', + 'external_converters', + ); + } + + protected async removeJS(name: string, module: ModuleExports): Promise { + for (const definition of this.getDefinitions(module)) { + // TODO: implement in ZHC + removeDefinition(definition); + } + } - for (const file of settings.get().external_converters) { + protected async loadJS(name: string, module: ModuleExports): Promise { + for (const definition of this.getDefinitions(module)) { try { - for (const definition of loadExternalConverter(file)) { - zhc.addDefinition(definition); - } - logger.info(`Loaded external converter '${file}'`); + // TODO: `updateDefinition` in ZHC instead? (add if not exist, replace if exist) + removeDefinition(definition); + addDefinition(definition); + logger.info(`Loaded external converter '${name}'.`); } catch (error) { - logger.error(`Failed to load external converter file '${file}' (${(error as Error).message})`); + logger.error(`Failed to load external converter '${name}' (${(error as Error).message})`); + logger.error(`Check the code for syntax error and make sure it is up to date with the current Zigbee2MQTT version.`); logger.error( - `Probably there is a syntax error in the file or the external converter is not ` + - `compatible with the current Zigbee2MQTT version`, - ); - logger.error( - `Note that external converters are not meant for long term usage, it's meant for local ` + - `testing after which a pull request should be created to add out-of-the-box support for the device`, + `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`, ); } } } + + private getDefinitions(module: ModuleExports): ExternalDefinition[] { + return Array.isArray(module) ? module : [module]; + } } diff --git a/lib/extension/externalExtension.ts b/lib/extension/externalExtension.ts deleted file mode 100644 index 6df6568554..0000000000 --- a/lib/extension/externalExtension.ts +++ /dev/null @@ -1,120 +0,0 @@ -import fs from 'fs'; -import path from 'path'; - -import bind from 'bind-decorator'; -import stringify from 'json-stable-stringify-without-jsonify'; - -import * as settings from '../util/settings'; -import utils from '../util/utils'; -import data from './../util/data'; -import logger from './../util/logger'; -import Extension from './extension'; - -const requestRegex = new RegExp(`${settings.get().mqtt.base_topic}/bridge/request/extension/(save|remove)`); - -export default class ExternalExtension extends Extension { - private requestLookup: {[s: string]: (message: KeyValue) => Promise} = { - save: this.saveExtension, - remove: this.removeExtension, - }; - - override async start(): Promise { - this.eventBus.onMQTTMessage(this, this.onMQTTMessage); - await this.loadUserDefinedExtensions(); - await this.publishExtensions(); - } - - private getExtensionsBasePath(): string { - return data.joinPath('extension'); - } - - private getListOfUserDefinedExtensions(): {name: string; code: string}[] { - const basePath = this.getExtensionsBasePath(); - if (fs.existsSync(basePath)) { - return fs - .readdirSync(basePath) - .filter((f) => f.endsWith('.js')) - .map((fileName) => { - const extensionFilePath = path.join(basePath, fileName); - return {name: fileName, code: fs.readFileSync(extensionFilePath, 'utf-8')}; - }); - } else { - return []; - } - } - - @bind private async removeExtension(message: KeyValue): Promise { - const {name} = message; - const extensions = this.getListOfUserDefinedExtensions(); - const extensionToBeRemoved = extensions.find((e) => e.name === name); - - if (extensionToBeRemoved) { - await this.enableDisableExtension(false, extensionToBeRemoved.name); - const basePath = this.getExtensionsBasePath(); - const extensionFilePath = path.join(basePath, path.basename(name)); - fs.unlinkSync(extensionFilePath); - await this.publishExtensions(); - logger.info(`Extension ${name} removed`); - return utils.getResponse(message, {}); - } else { - return utils.getResponse(message, {}, `Extension ${name} doesn't exists`); - } - } - - @bind private async saveExtension(message: KeyValue): Promise { - const {name, code} = message; - const ModuleConstructor = utils.loadModuleFromText(code, name) as typeof Extension; - await this.loadExtension(ModuleConstructor); - const basePath = this.getExtensionsBasePath(); - /* istanbul ignore else */ - if (!fs.existsSync(basePath)) { - fs.mkdirSync(basePath); - } - const extensionFilePath = path.join(basePath, path.basename(name)); - fs.writeFileSync(extensionFilePath, code); - await this.publishExtensions(); - logger.info(`Extension ${name} loaded`); - return utils.getResponse(message, {}); - } - - @bind async onMQTTMessage(data: eventdata.MQTTMessage): Promise { - const match = data.topic.match(requestRegex); - if (match && this.requestLookup[match[1].toLowerCase()]) { - const message = utils.parseJSON(data.message, data.message) as KeyValue; - try { - const response = await this.requestLookup[match[1].toLowerCase()](message); - await this.mqtt.publish(`bridge/response/extension/${match[1]}`, stringify(response)); - } catch (error) { - logger.error(`Request '${data.topic}' failed with error: '${(error as Error).message}'`); - const response = utils.getResponse(message, {}, `${(error as Error).message}`); - await this.mqtt.publish(`bridge/response/extension/${match[1]}`, stringify(response)); - } - } - } - - @bind private async loadExtension(ConstructorClass: typeof Extension): Promise { - await this.enableDisableExtension(false, ConstructorClass.name); - // @ts-expect-error `ConstructorClass` is the interface, not the actual passed class - await this.addExtension(new ConstructorClass(this.zigbee, this.mqtt, this.state, this.publishEntityState, this.eventBus, settings, logger)); - } - - private async loadUserDefinedExtensions(): Promise { - for (const extension of this.getListOfUserDefinedExtensions()) { - await this.loadExtension(utils.loadModuleFromText(extension.code, extension.name) as typeof Extension); - } - } - - private async publishExtensions(): Promise { - const extensions = this.getListOfUserDefinedExtensions(); - await this.mqtt.publish( - 'bridge/extensions', - stringify(extensions), - { - retain: true, - qos: 0, - }, - settings.get().mqtt.base_topic, - true, - ); - } -} diff --git a/lib/extension/externalExtensions.ts b/lib/extension/externalExtensions.ts new file mode 100644 index 0000000000..3537617b34 --- /dev/null +++ b/lib/extension/externalExtensions.ts @@ -0,0 +1,59 @@ +import type Extension from './extension'; + +import logger from '../util/logger'; +import * as settings from '../util/settings'; +import ExternalJSExtension from './externalJS'; + +type ModuleExports = typeof Extension; + +export default class ExternalExtensions extends ExternalJSExtension { + constructor( + zigbee: Zigbee, + mqtt: MQTT, + state: State, + publishEntityState: PublishEntityState, + eventBus: EventBus, + enableDisableExtension: (enable: boolean, name: string) => Promise, + restartCallback: () => Promise, + addExtension: (extension: Extension) => Promise, + ) { + super( + zigbee, + mqtt, + state, + publishEntityState, + eventBus, + enableDisableExtension, + restartCallback, + addExtension, + 'extension', + 'external_extensions', + ); + } + + protected async removeJS(name: string, module: ModuleExports): Promise { + await this.enableDisableExtension(false, module.name); + } + + protected async loadJS(name: string, module: ModuleExports): Promise { + // stop if already started + await this.enableDisableExtension(false, module.name); + await this.addExtension( + // @ts-expect-error `module` is the interface, not the actual passed class + new module( + this.zigbee, + this.mqtt, + this.state, + this.publishEntityState, + this.eventBus, + this.enableDisableExtension, + this.restartCallback, + this.addExtension, + settings, + logger, + ), + ); + + logger.info(`Loaded external extension '${name}'.`); + } +} diff --git a/lib/extension/externalJS.ts b/lib/extension/externalJS.ts new file mode 100644 index 0000000000..f496804a6a --- /dev/null +++ b/lib/extension/externalJS.ts @@ -0,0 +1,162 @@ +import fs from 'fs'; +import path from 'path'; +import {Context, runInNewContext} from 'vm'; + +import bind from 'bind-decorator'; +import stringify from 'json-stable-stringify-without-jsonify'; +import data from 'lib/util/data'; +import utils from 'lib/util/utils'; + +import * as settings from '../util/settings'; +import logger from './../util/logger'; +import Extension from './extension'; + +export default abstract class ExternalJSExtension extends Extension { + private requestLookup: {[s: string]: (message: KeyValue) => Promise} = { + save: this.save, + remove: this.remove, + }; + + protected mqttTopic: string; + protected requestRegex: RegExp; + protected basePath: string; + + constructor( + zigbee: Zigbee, + mqtt: MQTT, + state: State, + publishEntityState: PublishEntityState, + eventBus: EventBus, + enableDisableExtension: (enable: boolean, name: string) => Promise, + restartCallback: () => Promise, + addExtension: (extension: Extension) => Promise, + mqttTopic: string, + folderName: string, + ) { + super(zigbee, mqtt, state, publishEntityState, eventBus, enableDisableExtension, restartCallback, addExtension); + + this.mqttTopic = mqttTopic; + this.requestRegex = new RegExp(`${settings.get().mqtt.base_topic}/bridge/request/${mqttTopic}/(save|remove)`); + this.basePath = data.joinPath(folderName); + } + + override async start(): Promise { + await super.start(); + this.eventBus.onMQTTMessage(this, this.onMQTTMessage); + await this.loadFiles(); + await this.publishExternalJS(); + } + + private getFilePath(name: string, mkBasePath: boolean = false): string { + if (mkBasePath && !fs.existsSync(this.basePath)) { + fs.mkdirSync(this.basePath, {recursive: true}); + } + + return path.join(this.basePath, name); + } + + protected getFileCode(name: string): string { + return fs.readFileSync(path.join(this.basePath, name), 'utf8'); + } + + protected *getFiles(): Generator<{name: string; code: string}> { + for (const fileName of fs.readdirSync(this.basePath)) { + if (fileName.endsWith('.js')) { + yield {name: fileName, code: this.getFileCode(fileName)}; + } + } + } + + @bind async onMQTTMessage(data: eventdata.MQTTMessage): Promise { + const match = data.topic.match(this.requestRegex); + + if (match && this.requestLookup[match[1].toLowerCase()]) { + const message = utils.parseJSON(data.message, data.message) as KeyValue; + + try { + const response = await this.requestLookup[match[1].toLowerCase()](message); + + await this.mqtt.publish(`bridge/response/${this.mqttTopic}/${match[1]}`, stringify(response)); + } catch (error) { + logger.error(`Request '${data.topic}' failed with error: '${(error as Error).message}'`); + + const response = utils.getResponse(message, {}, `${(error as Error).message}`); + + await this.mqtt.publish(`bridge/response/${this.mqttTopic}/${match[1]}`, stringify(response)); + } + } + } + + protected abstract removeJS(name: string, module: M): Promise; + + protected abstract loadJS(name: string, module: M): Promise; + + @bind private async remove(message: KeyValue): Promise { + const {name} = message; + const toBeRemoved = this.getFilePath(name); + + if (fs.existsSync(toBeRemoved)) { + await this.removeJS(name, this.loadModuleFromText(this.getFileCode(name), name)); + + 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`); + } + } + + @bind private async save(message: KeyValue): Promise { + const {name, code} = message; + + await this.loadJS(name, this.loadModuleFromText(code, name)); + + const filePath = this.getFilePath(name, true); + + fs.writeFileSync(filePath, code, 'utf8'); + logger.info(`${name} loaded. Contents written to '${filePath}'.`); + await this.publishExternalJS(); + + return utils.getResponse(message, {}); + } + + private async loadFiles(): Promise { + for (const extension of this.getFiles()) { + await this.loadJS(extension.name, this.loadModuleFromText(extension.code, extension.name)); + } + } + + private async publishExternalJS(): Promise { + await this.mqtt.publish( + `bridge/${this.mqttTopic}s`, + stringify(Array.from(this.getFiles())), + { + retain: true, + qos: 0, + }, + settings.get().mqtt.base_topic, + true, + ); + } + + private loadModuleFromText(moduleCode: string, name?: string): M { + const moduleFakePath = path.join(__dirname, '..', '..', 'data', 'extension', name || 'externally-loaded.js'); + const sandbox: Context = { + require: require, + module: {}, + console, + setTimeout, + clearTimeout, + setInterval, + clearInterval, + setImmediate, + clearImmediate, + }; + + runInNewContext(moduleCode, sandbox, moduleFakePath); + + return sandbox.module.exports; + } +} diff --git a/lib/util/utils.ts b/lib/util/utils.ts index f44e921581..08a5703a2e 100644 --- a/lib/util/utils.ts +++ b/lib/util/utils.ts @@ -3,13 +3,10 @@ import type * as zhc from 'zigbee-herdsman-converters'; import assert from 'assert'; import fs from 'fs'; import path from 'path'; -import vm from 'vm'; import equals from 'fast-deep-equal/es6'; import humanizeDuration from 'humanize-duration'; -import data from './data'; - function pad(num: number): string { const norm = Math.floor(Math.abs(num)); return (norm < 10 ? '0' : '') + norm; @@ -145,48 +142,6 @@ function parseJSON(value: string, fallback: string): KeyValue | string { } } -function loadModuleFromText(moduleCode: string, name?: string): unknown { - const moduleFakePath = path.join(__dirname, '..', '..', 'data', 'extension', name || 'externally-loaded.js'); - const sandbox = { - require: require, - module: {}, - console, - setTimeout, - clearTimeout, - setInterval, - clearInterval, - setImmediate, - clearImmediate, - }; - vm.runInNewContext(moduleCode, sandbox, moduleFakePath); - /* eslint-disable-line */ // @ts-ignore - return sandbox.module.exports; -} - -function loadModuleFromFile(modulePath: string): unknown { - const moduleCode = fs.readFileSync(modulePath, {encoding: 'utf8'}); - return loadModuleFromText(moduleCode); -} - -export function* loadExternalConverter(moduleName: string): Generator { - let converter; - - if (moduleName.endsWith('.js')) { - converter = loadModuleFromFile(data.joinPath(moduleName)); - } else { - // eslint-disable-next-line @typescript-eslint/no-require-imports - converter = require(moduleName); - } - - if (Array.isArray(converter)) { - for (const item of converter) { - yield item; - } - } else { - yield converter; - } -} - /** * Delete all keys from passed object that have null/undefined values. * @@ -426,8 +381,6 @@ export default { getObjectProperty, getResponse, parseJSON, - loadModuleFromText, - loadModuleFromFile, removeNullPropertiesFromObject, toNetworkAddressHex, toSnakeCaseString, From 8d515f605c37f76430de39b2d69d13d1ecb06962 Mon Sep 17 00:00:00 2001 From: Nerivec <62446222+Nerivec@users.noreply.github.com> Date: Thu, 14 Nov 2024 00:07:07 +0100 Subject: [PATCH 12/26] Fix imports --- lib/extension/externalJS.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/extension/externalJS.ts b/lib/extension/externalJS.ts index f496804a6a..073634cda6 100644 --- a/lib/extension/externalJS.ts +++ b/lib/extension/externalJS.ts @@ -4,11 +4,11 @@ import {Context, runInNewContext} from 'vm'; import bind from 'bind-decorator'; import stringify from 'json-stable-stringify-without-jsonify'; -import data from 'lib/util/data'; -import utils from 'lib/util/utils'; +import data from '../util/data'; +import logger from '../util/logger'; import * as settings from '../util/settings'; -import logger from './../util/logger'; +import utils from '../util/utils'; import Extension from './extension'; export default abstract class ExternalJSExtension extends Extension { From 20b7f6f47fb3afd153c61aca07ec02449e0d6672 Mon Sep 17 00:00:00 2001 From: Nerivec <62446222+Nerivec@users.noreply.github.com> Date: Thu, 14 Nov 2024 00:59:46 +0100 Subject: [PATCH 13/26] Improve error message on MQTT save --- lib/extension/externalJS.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/lib/extension/externalJS.ts b/lib/extension/externalJS.ts index 073634cda6..4cc951c0af 100644 --- a/lib/extension/externalJS.ts +++ b/lib/extension/externalJS.ts @@ -111,15 +111,19 @@ export default abstract class ExternalJSExtension extends Extension { @bind private async save(message: KeyValue): Promise { const {name, code} = message; - await this.loadJS(name, this.loadModuleFromText(code, name)); + try { + await this.loadJS(name, this.loadModuleFromText(code, name)); - const filePath = this.getFilePath(name, true); + const filePath = this.getFilePath(name, true); - fs.writeFileSync(filePath, code, 'utf8'); - logger.info(`${name} loaded. Contents written to '${filePath}'.`); - await this.publishExternalJS(); + fs.writeFileSync(filePath, code, 'utf8'); + logger.info(`${name} loaded. Contents written to '${filePath}'.`); + await this.publishExternalJS(); - return utils.getResponse(message, {}); + return utils.getResponse(message, {}); + } catch (error) { + return utils.getResponse(message, {}, `${name} contains invalid code: ${(error as Error).message}`); + } } private async loadFiles(): Promise { From f62e0ab259b36039276320eb2caa4ad001dbccff Mon Sep 17 00:00:00 2001 From: Nerivec <62446222+Nerivec@users.noreply.github.com> Date: Thu, 14 Nov 2024 01:19:23 +0100 Subject: [PATCH 14/26] Handle non-existing base path --- lib/extension/externalJS.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/extension/externalJS.ts b/lib/extension/externalJS.ts index 4cc951c0af..4b997269f3 100644 --- a/lib/extension/externalJS.ts +++ b/lib/extension/externalJS.ts @@ -60,6 +60,10 @@ export default abstract class ExternalJSExtension extends Extension { } protected *getFiles(): Generator<{name: string; code: string}> { + if (!fs.existsSync(this.basePath)) { + return; + } + for (const fileName of fs.readdirSync(this.basePath)) { if (fileName.endsWith('.js')) { yield {name: fileName, code: this.getFileCode(fileName)}; From f13789b81b75607a6b4de554eeed1a4f6d45e3ab Mon Sep 17 00:00:00 2001 From: Nerivec <62446222+Nerivec@users.noreply.github.com> Date: Thu, 14 Nov 2024 01:43:53 +0100 Subject: [PATCH 15/26] Throw on bad converter --- lib/extension/externalConverters.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/extension/externalConverters.ts b/lib/extension/externalConverters.ts index 910b1cdf64..a958c3d33c 100644 --- a/lib/extension/externalConverters.ts +++ b/lib/extension/externalConverters.ts @@ -45,11 +45,13 @@ export default class ExternalConverters extends ExternalJSExtension Date: Thu, 14 Nov 2024 01:56:17 +0100 Subject: [PATCH 16/26] Add tests --- .../mock-external-converter-multiple.js | 0 .../mock-external-converter.js | 0 .../external_extensions/example2Extension.js | 16 + .../exampleExtension.js | 6 +- test/extensions/externalConverters.test.ts | 286 ++++++++++++------ test/extensions/externalExtension.test.ts | 134 -------- test/extensions/externalExtensions.test.ts | 209 +++++++++++++ 7 files changed, 417 insertions(+), 234 deletions(-) rename test/assets/{ => external_converters}/mock-external-converter-multiple.js (100%) rename test/assets/{ => external_converters}/mock-external-converter.js (100%) create mode 100644 test/assets/external_extensions/example2Extension.js rename test/assets/{ => external_extensions}/exampleExtension.js (61%) delete mode 100644 test/extensions/externalExtension.test.ts create mode 100644 test/extensions/externalExtensions.test.ts diff --git a/test/assets/mock-external-converter-multiple.js b/test/assets/external_converters/mock-external-converter-multiple.js similarity index 100% rename from test/assets/mock-external-converter-multiple.js rename to test/assets/external_converters/mock-external-converter-multiple.js diff --git a/test/assets/mock-external-converter.js b/test/assets/external_converters/mock-external-converter.js similarity index 100% rename from test/assets/mock-external-converter.js rename to test/assets/external_converters/mock-external-converter.js diff --git a/test/assets/external_extensions/example2Extension.js b/test/assets/external_extensions/example2Extension.js new file mode 100644 index 0000000000..258de01b71 --- /dev/null +++ b/test/assets/external_extensions/example2Extension.js @@ -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'); + } +} + +module.exports = Example2; diff --git a/test/assets/exampleExtension.js b/test/assets/external_extensions/exampleExtension.js similarity index 61% rename from test/assets/exampleExtension.js rename to test/assets/external_extensions/exampleExtension.js index 149cc96f48..aec02a8934 100644 --- a/test/assets/exampleExtension.js +++ b/test/assets/external_extensions/exampleExtension.js @@ -5,7 +5,11 @@ class Example { } start() { - this.mqtt.publish('example/extension', 'test'); + this.mqtt.publish('example/extension', 'call from start'); + } + + stop() { + this.mqtt.publish('example/extension', 'call from stop'); } } diff --git a/test/extensions/externalConverters.test.ts b/test/extensions/externalConverters.test.ts index 54308cb90f..f54603862e 100644 --- a/test/extensions/externalConverters.test.ts +++ b/test/extensions/externalConverters.test.ts @@ -1,119 +1,109 @@ import * as data from '../mocks/data'; import {mockLogger} from '../mocks/logger'; -import {mockMQTT} from '../mocks/mqtt'; +import {mockMQTT, events as mockMQTTEvents} from '../mocks/mqtt'; import {flushPromises} from '../mocks/utils'; -import {devices, mockController as mockZHController} from '../mocks/zigbeeHerdsman'; +import {devices, mockController as mockZHController, returnDevices} from '../mocks/zigbeeHerdsman'; import fs from 'fs'; import path from 'path'; +import stringify from 'json-stable-stringify-without-jsonify'; + import * as zhc from 'zigbee-herdsman-converters'; import {Controller} from '../../lib/controller'; import * as settings from '../../lib/util/settings'; -jest.mock( - 'mock-external-converter-module', - () => { - return { - mock: true, - }; - }, - { - virtual: true, - }, -); - -jest.mock( - 'mock-multiple-external-converter-module', - () => { - return [ - { - mock: 1, - }, - { - mock: 2, - }, - ]; - }, - { - virtual: true, - }, -); +const BASE_DIR = 'external_converters'; const mockZHCAddDefinition = jest.fn(); +const mockZHCRemoveDefinition = jest.fn(); // @ts-expect-error mock zhc.addDefinition = mockZHCAddDefinition; - -const mocksClear = [ - mockZHCAddDefinition, - devices.bulb_color.removeFromNetwork, - devices.bulb.removeFromNetwork, - mockZHController.permitJoin, - mockZHController.stop, - mockMQTT.endAsync, - mockMQTT.publishAsync, - mockLogger.debug, - mockLogger.error, -]; +// @ts-expect-error mock +zhc.removeDefinition = mockZHCRemoveDefinition; describe('Extension: ExternalConverters', () => { + const mockBasePath = path.join(data.mockDir, BASE_DIR); let controller: Controller; - const resetExtension = async (): Promise => { - await controller.enableDisableExtension(false, 'ExternalConverters'); - await controller.enableDisableExtension(true, 'ExternalConverters'); + const existsSyncSpy = jest.spyOn(fs, 'existsSync'); + const readdirSyncSpy = jest.spyOn(fs, 'readdirSync'); + const mkdirSyncSpy = jest.spyOn(fs, 'mkdirSync'); + const rmSyncSpy = jest.spyOn(fs, 'rmSync'); + const writeFileSyncSpy = jest.spyOn(fs, 'writeFileSync'); + + const mocksClear = [ + mockMQTT.endAsync, + mockMQTT.publishAsync, + mockLogger.debug, + mockLogger.error, + mockZHController.stop, + devices.bulb.save, + mockZHCAddDefinition, + mockZHCRemoveDefinition, + existsSyncSpy, + readdirSyncSpy, + mkdirSyncSpy, + rmSyncSpy, + writeFileSyncSpy, + ]; + + const useAssets = (): void => { + fs.cpSync(path.join(__dirname, '..', 'assets', BASE_DIR), mockBasePath, {recursive: true}); + }; + + const getFileCode = (fileName: string): string => { + return fs.readFileSync(path.join(__dirname, '..', 'assets', BASE_DIR, fileName), 'utf8'); }; beforeAll(async () => { jest.useFakeTimers(); - controller = new Controller(jest.fn(), jest.fn()); - await controller.start(); - await flushPromises(); + }); + + afterAll(async () => { + jest.useRealTimers(); }); beforeEach(async () => { + mocksClear.forEach((m) => m.mockClear()); data.writeDefaultConfiguration(); + data.writeDefaultState(); settings.reRead(); - mocksClear.forEach((m) => m.mockClear()); - await resetExtension(); - }); + returnDevices.splice(0); - afterAll(async () => { - jest.useRealTimers(); + controller = new Controller(jest.fn(), jest.fn()); }); - it('Does not load external converters', async () => { - settings.set(['external_converters'], []); - await resetExtension(); - expect(mockZHCAddDefinition).toHaveBeenCalledTimes(0); + afterEach(() => { + fs.rmSync(mockBasePath, {recursive: true, force: true}); }); - it('Loads external converters', async () => { - fs.copyFileSync(path.join(__dirname, '..', 'assets', 'mock-external-converter.js'), path.join(data.mockDir, 'mock-external-converter.js')); - settings.set(['external_converters'], ['mock-external-converter.js']); - await resetExtension(); - expect(mockZHCAddDefinition).toHaveBeenCalledTimes(1); - expect(mockZHCAddDefinition).toHaveBeenCalledWith({ - mock: true, - zigbeeModel: ['external_converter_device'], - vendor: 'external', - model: 'external_converter_device', - description: 'external', - fromZigbee: [], - toZigbee: [], - exposes: [], - }); + it('loads nothing from folder', async () => { + await controller.start(); + await flushPromises(); + + expect(existsSyncSpy).toHaveBeenCalledWith(mockBasePath); + expect(readdirSyncSpy).not.toHaveBeenCalledWith(mockBasePath); + expect(mockMQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bridge/converters', stringify([]), {retain: true, qos: 0}, expect.any(Function)); }); - it('Loads multiple external converters', async () => { - fs.copyFileSync( - path.join(__dirname, '..', 'assets', 'mock-external-converter-multiple.js'), - path.join(data.mockDir, 'mock-external-converter-multiple.js'), + it('loads from folder', async () => { + useAssets(); + + await controller.start(); + await flushPromises(); + + expect(mockMQTT.publish).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')}, + ]), + {retain: true, qos: 0}, + expect.any(Function), ); - settings.set(['external_converters'], ['mock-external-converter-multiple.js']); - await resetExtension(); - expect(mockZHCAddDefinition).toHaveBeenCalledTimes(2); + expect(mockZHCRemoveDefinition).toHaveBeenCalledTimes(3); expect(mockZHCAddDefinition).toHaveBeenNthCalledWith(1, { mock: 1, model: 'external_converters_device_1', @@ -134,36 +124,134 @@ describe('Extension: ExternalConverters', () => { toZigbee: [], exposes: [], }); + expect(mockZHCAddDefinition).toHaveBeenNthCalledWith(3, { + mock: true, + zigbeeModel: ['external_converter_device'], + vendor: 'external', + model: 'external_converter_device', + description: 'external', + fromZigbee: [], + toZigbee: [], + exposes: [], + }); }); - it('Loads external converters from package', async () => { - settings.set(['external_converters'], ['mock-external-converter-module']); - await resetExtension(); - expect(mockZHCAddDefinition).toHaveBeenCalledTimes(1); + it('saves and removes from MQTT', async () => { + const converterName = 'foo.js'; + const converterCode = getFileCode('mock-external-converter.js'); + const converterFilePath = path.join(mockBasePath, converterName); + + await controller.start(); + await flushPromises(); + mocksClear.forEach((m) => m.mockClear()); + + //-- SAVE + mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/save', stringify({name: converterName, code: converterCode})); + await flushPromises(); + + expect(mkdirSyncSpy).toHaveBeenCalledWith(mockBasePath, {recursive: true}); + expect(writeFileSyncSpy).toHaveBeenCalledWith(converterFilePath, converterCode, 'utf8'); + expect(mockZHCRemoveDefinition).toHaveBeenCalledTimes(1); expect(mockZHCAddDefinition).toHaveBeenCalledWith({ mock: true, + zigbeeModel: ['external_converter_device'], + vendor: 'external', + model: 'external_converter_device', + description: 'external', + fromZigbee: [], + toZigbee: [], + exposes: [], }); - }); + expect(mockMQTT.publish).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/converters', + stringify([{name: converterName, code: converterCode}]), + {retain: true, qos: 0}, + expect.any(Function), + ); - it('Loads multiple external converters from package', async () => { - settings.set(['external_converters'], ['mock-multiple-external-converter-module']); - await resetExtension(); - expect(mockZHCAddDefinition).toHaveBeenCalledTimes(2); - expect(mockZHCAddDefinition).toHaveBeenNthCalledWith(1, { - mock: 1, - }); - expect(mockZHCAddDefinition).toHaveBeenNthCalledWith(2, { - mock: 2, + //-- REMOVE + mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/remove', stringify({name: converterName})); + await flushPromises(); + + expect(rmSyncSpy).toHaveBeenCalledWith(converterFilePath, {force: true}); + expect(mockZHCRemoveDefinition).toHaveBeenCalledWith({ + mock: true, + zigbeeModel: ['external_converter_device'], + vendor: 'external', + model: 'external_converter_device', + description: 'external', + fromZigbee: [], + toZigbee: [], + exposes: [], }); + expect(mockMQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bridge/converters', stringify([]), {retain: true, qos: 0}, expect.any(Function)); + }); + + 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 controller.start(); + await flushPromises(); + mocksClear.forEach((m) => m.mockClear()); + + mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/save', stringify({name: converterName, code: converterCode})); + await flushPromises(); + + expect(mockMQTT.publish).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/response/converter/save', + expect.stringContaining(`"error":"foo.js contains invalid code`), + {retain: false, qos: 0}, + expect.any(Function), + ); + expect(writeFileSyncSpy).not.toHaveBeenCalledWith(converterFilePath, converterCode, 'utf8'); }); - it('Loads external converters with error', async () => { - fs.copyFileSync(path.join(__dirname, '..', 'assets', 'mock-external-converter.js'), path.join(data.mockDir, 'mock-external-converter.js')); - settings.set(['external_converters'], ['mock-external-converter.js']); + it('returns error on invalid removal', async () => { + const converterName = 'invalid.js'; + const converterFilePath = path.join(mockBasePath, converterName); + + await controller.start(); + await flushPromises(); + mocksClear.forEach((m) => m.mockClear()); + + mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/remove', stringify({name: converterName})); + await flushPromises(); + + expect(mockMQTT.publish).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/response/converter/remove', + stringify({data: {}, status: 'error', error: `${converterName} (${converterFilePath}) doesn't exists`}), + {retain: false, qos: 0}, + expect.any(Function), + ); + expect(rmSyncSpy).not.toHaveBeenCalledWith(converterFilePath, {force: true}); + }); + + it('returns error on invalid definition', async () => { + const converterName = 'foo.js'; + const converterCode = getFileCode('mock-external-converter.js'); + const converterFilePath = path.join(mockBasePath, converterName); + + await controller.start(); + await flushPromises(); + mocksClear.forEach((m) => m.mockClear()); + + const errorMsg = `Invalid definition`; + mockZHCAddDefinition.mockImplementationOnce(() => { - throw new Error('Invalid definition!'); + throw new Error(errorMsg); }); - await resetExtension(); - expect(mockLogger.error).toHaveBeenCalledWith(`Failed to load external converter file 'mock-external-converter.js' (Invalid definition!)`); + + mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/save', stringify({name: converterName, code: converterCode})); + await flushPromises(); + + expect(mockMQTT.publish).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/response/converter/save', + expect.stringContaining(errorMsg), + {retain: false, qos: 0}, + expect.any(Function), + ); + expect(writeFileSyncSpy).not.toHaveBeenCalledWith(converterFilePath, converterCode, 'utf8'); }); }); diff --git a/test/extensions/externalExtension.test.ts b/test/extensions/externalExtension.test.ts deleted file mode 100644 index 1e6c9b1b45..0000000000 --- a/test/extensions/externalExtension.test.ts +++ /dev/null @@ -1,134 +0,0 @@ -import * as data from '../mocks/data'; -import {mockLogger} from '../mocks/logger'; -import {mockMQTT, events as mockMQTTEvents} from '../mocks/mqtt'; -import {flushPromises} from '../mocks/utils'; -import {devices, mockController as mockZHController, returnDevices} from '../mocks/zigbeeHerdsman'; - -import fs from 'fs'; -import path from 'path'; - -import stringify from 'json-stable-stringify-without-jsonify'; -import {rimrafSync} from 'rimraf'; - -import {Controller} from '../../lib/controller'; -import * as settings from '../../lib/util/settings'; - -const mocksClear = [ - mockZHController.permitJoin, - mockZHController.stop, - devices.bulb_color.removeFromNetwork, - devices.bulb.removeFromNetwork, - mockMQTT.endAsync, - mockMQTT.publishAsync, - mockLogger.debug, - mockLogger.error, -]; - -describe('Extension: ExternalExtension', () => { - let controller: Controller; - let mkdirSyncSpy: jest.SpyInstance; - let unlinkSyncSpy: jest.SpyInstance; - - beforeAll(async () => { - jest.useFakeTimers(); - mkdirSyncSpy = jest.spyOn(fs, 'mkdirSync'); - unlinkSyncSpy = jest.spyOn(fs, 'unlinkSync'); - }); - - afterAll(async () => { - jest.useRealTimers(); - }); - - beforeEach(async () => { - data.writeDefaultConfiguration(); - settings.reRead(); - mocksClear.forEach((m) => m.mockClear()); - returnDevices.splice(0); - mocksClear.forEach((m) => m.mockClear()); - data.writeDefaultConfiguration(); - settings.reRead(); - data.writeDefaultState(); - }); - - afterEach(() => { - const extensionPath = path.join(data.mockDir, 'extension'); - rimrafSync(extensionPath); - }); - - it('Load user extension', async () => { - const extensionPath = path.join(data.mockDir, 'extension'); - const extensionCode = fs.readFileSync(path.join(__dirname, '..', 'assets', 'exampleExtension.js'), 'utf-8'); - fs.mkdirSync(extensionPath); - fs.copyFileSync(path.join(__dirname, '..', 'assets', 'exampleExtension.js'), path.join(extensionPath, 'exampleExtension.js')); - controller = new Controller(jest.fn(), jest.fn()); - await controller.start(); - await flushPromises(); - expect(mockMQTT.publishAsync).toHaveBeenCalledWith('zigbee2mqtt/example/extension', 'test', {retain: false, qos: 0}); - expect(mockMQTT.publishAsync).toHaveBeenCalledWith( - 'zigbee2mqtt/bridge/extensions', - stringify([{name: 'exampleExtension.js', code: extensionCode}]), - {retain: true, qos: 0}, - ); - }); - - it('Load user extension from api call', async () => { - const extensionPath = path.join(data.mockDir, 'extension'); - const extensionCode = fs.readFileSync(path.join(__dirname, '..', 'assets', 'exampleExtension.js'), 'utf-8'); - controller = new Controller(jest.fn(), jest.fn()); - await controller.start(); - await flushPromises(); - mockMQTT.publishAsync.mockClear(); - mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/save', stringify({name: 'foo.js', code: extensionCode})); - await flushPromises(); - expect(mockMQTT.publishAsync).toHaveBeenCalledWith('zigbee2mqtt/bridge/extensions', stringify([{name: 'foo.js', code: extensionCode}]), { - retain: true, - qos: 0, - }); - expect(mockMQTT.publishAsync).toHaveBeenCalledWith('zigbee2mqtt/example/extension', 'call from constructor', {retain: false, qos: 0}); - expect(mkdirSyncSpy).toHaveBeenCalledWith(extensionPath); - }); - - it('Do not load corrupted extensions', async () => { - const extensionCode = 'definetly not a correct javascript code'; - controller = new Controller(jest.fn(), jest.fn()); - await controller.start(); - await flushPromises(); - mockMQTT.publishAsync.mockClear(); - mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/save', stringify({name: 'foo.js', code: extensionCode})); - await flushPromises(); - - expect(mockMQTT.publishAsync).toHaveBeenCalledWith('zigbee2mqtt/bridge/response/extension/save', expect.any(String), {retain: false, qos: 0}); - const payload = JSON.parse(mockMQTT.publishAsync.mock.calls[0][1]); - expect(payload).toEqual(expect.objectContaining({data: {}, status: 'error'})); - expect(payload.error).toMatch('Unexpected identifier'); - }); - - it('Removes user extension', async () => { - const extensionPath = path.join(data.mockDir, 'extension'); - const extensionCode = fs.readFileSync(path.join(__dirname, '..', 'assets', 'exampleExtension.js'), 'utf-8'); - fs.mkdirSync(extensionPath); - const extensionFilePath = path.join(extensionPath, 'exampleExtension.js'); - fs.copyFileSync(path.join(__dirname, '..', 'assets', 'exampleExtension.js'), extensionFilePath); - controller = new Controller(jest.fn(), jest.fn()); - await controller.start(); - await flushPromises(); - expect(mockMQTT.publishAsync).toHaveBeenCalledWith('zigbee2mqtt/example/extension', 'test', {retain: false, qos: 0}); - expect(mockMQTT.publishAsync).toHaveBeenCalledWith( - 'zigbee2mqtt/bridge/extensions', - stringify([{name: 'exampleExtension.js', code: extensionCode}]), - {retain: true, qos: 0}, - ); - - mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/remove', stringify({name: 'exampleExtension.js'})); - await flushPromises(); - expect(unlinkSyncSpy).toHaveBeenCalledWith(extensionFilePath); - mockMQTT.publishAsync.mockClear(); - mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/remove', stringify({name: 'non existing.js'})); - await flushPromises(); - expect(mockMQTT.publishAsync).toHaveBeenCalledWith( - 'zigbee2mqtt/bridge/response/extension/remove', - stringify({data: {}, status: 'error', error: "Extension non existing.js doesn't exists"}), - {retain: false, qos: 0}, - ); - }); -}); diff --git a/test/extensions/externalExtensions.test.ts b/test/extensions/externalExtensions.test.ts new file mode 100644 index 0000000000..ee5d654fd9 --- /dev/null +++ b/test/extensions/externalExtensions.test.ts @@ -0,0 +1,209 @@ +import * as data from '../mocks/data'; +import {mockLogger} from '../mocks/logger'; +import {mockMQTT, events as mockMQTTEvents} from '../mocks/mqtt'; +import {flushPromises} from '../mocks/utils'; +import {devices, mockController as mockZHController, returnDevices} from '../mocks/zigbeeHerdsman'; + +import fs from 'fs'; +import path from 'path'; + +import stringify from 'json-stable-stringify-without-jsonify'; + +import {Controller} from '../../lib/controller'; +import * as settings from '../../lib/util/settings'; + +const BASE_DIR = 'external_extensions'; + +describe('Extension: ExternalExtensions', () => { + let controller: Controller; + const mockBasePath = path.join(data.mockDir, BASE_DIR); + + const existsSyncSpy = jest.spyOn(fs, 'existsSync'); + const readdirSyncSpy = jest.spyOn(fs, 'readdirSync'); + const mkdirSyncSpy = jest.spyOn(fs, 'mkdirSync'); + const rmSyncSpy = jest.spyOn(fs, 'rmSync'); + const writeFileSyncSpy = jest.spyOn(fs, 'writeFileSync'); + + const mocksClear = [ + mockMQTT.end, + mockMQTT.publish, + mockLogger.debug, + mockLogger.error, + mockZHController.stop, + devices.bulb.save, + existsSyncSpy, + readdirSyncSpy, + mkdirSyncSpy, + rmSyncSpy, + writeFileSyncSpy, + ]; + + const useAssets = (): void => { + fs.cpSync(path.join(__dirname, '..', 'assets', BASE_DIR), mockBasePath, {recursive: true}); + }; + + const getFileCode = (fileName: string): string => { + return fs.readFileSync(path.join(__dirname, '..', 'assets', BASE_DIR, fileName), 'utf8'); + }; + + beforeAll(async () => { + jest.useFakeTimers(); + }); + + afterAll(async () => { + jest.useRealTimers(); + }); + + beforeEach(async () => { + mocksClear.forEach((m) => m.mockClear()); + data.writeDefaultConfiguration(); + data.writeDefaultState(); + settings.reRead(); + returnDevices.splice(0); + + controller = new Controller(jest.fn(), jest.fn()); + }); + + afterEach(() => { + fs.rmSync(mockBasePath, {recursive: true, force: true}); + }); + + it('loads nothing from folder', async () => { + await controller.start(); + await flushPromises(); + + expect(existsSyncSpy).toHaveBeenCalledWith(mockBasePath); + expect(readdirSyncSpy).not.toHaveBeenCalledWith(mockBasePath); + expect(mockMQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bridge/extensions', stringify([]), {retain: true, qos: 0}, expect.any(Function)); + }); + + it('loads from folder', async () => { + useAssets(); + + await controller.start(); + await flushPromises(); + + expect(mockMQTT.publish).toHaveBeenCalledWith( + 'zigbee2mqtt/example2/extension', + 'call2 from constructor', + {retain: false, qos: 0}, + expect.any(Function), + ); + expect(mockMQTT.publish).toHaveBeenCalledWith( + 'zigbee2mqtt/example2/extension', + 'call2 from start', + {retain: false, qos: 0}, + expect.any(Function), + ); + expect(mockMQTT.publish).toHaveBeenCalledWith( + 'zigbee2mqtt/example/extension', + 'call from constructor', + {retain: false, qos: 0}, + expect.any(Function), + ); + expect(mockMQTT.publish).toHaveBeenCalledWith( + 'zigbee2mqtt/example/extension', + 'call from start', + {retain: false, qos: 0}, + expect.any(Function), + ); + expect(mockMQTT.publish).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/extensions', + stringify([ + {name: 'example2Extension.js', code: getFileCode('example2Extension.js')}, + {name: 'exampleExtension.js', code: getFileCode('exampleExtension.js')}, + ]), + {retain: true, qos: 0}, + expect.any(Function), + ); + }); + + it('saves and removes from MQTT', async () => { + const extensionName = 'foo.js'; + const extensionCode = getFileCode('exampleExtension.js'); + const extensionFilePath = path.join(mockBasePath, extensionName); + + await controller.start(); + await flushPromises(); + mocksClear.forEach((m) => m.mockClear()); + + //-- SAVE + mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/save', stringify({name: extensionName, code: extensionCode})); + await flushPromises(); + + expect(mkdirSyncSpy).toHaveBeenCalledWith(mockBasePath, {recursive: true}); + expect(writeFileSyncSpy).toHaveBeenCalledWith(extensionFilePath, extensionCode, 'utf8'); + expect(mockMQTT.publish).toHaveBeenCalledWith( + 'zigbee2mqtt/example/extension', + 'call from constructor', + {retain: false, qos: 0}, + expect.any(Function), + ); + expect(mockMQTT.publish).toHaveBeenCalledWith( + 'zigbee2mqtt/example/extension', + 'call from start', + {retain: false, qos: 0}, + expect.any(Function), + ); + expect(mockMQTT.publish).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/extensions', + stringify([{name: extensionName, code: extensionCode}]), + {retain: true, qos: 0}, + expect.any(Function), + ); + + //-- REMOVE + mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/remove', stringify({name: extensionName})); + await flushPromises(); + + expect(rmSyncSpy).toHaveBeenCalledWith(extensionFilePath, {force: true}); + expect(mockMQTT.publish).toHaveBeenCalledWith( + 'zigbee2mqtt/example/extension', + 'call from stop', + {retain: false, qos: 0}, + expect.any(Function), + ); + expect(mockMQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bridge/extensions', stringify([]), {retain: true, qos: 0}, expect.any(Function)); + }); + + 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); + + await controller.start(); + await flushPromises(); + mocksClear.forEach((m) => m.mockClear()); + + mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/save', stringify({name: extensionName, code: extensionCode})); + await flushPromises(); + + expect(mockMQTT.publish).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/response/extension/save', + expect.stringContaining(`"error":"${extensionName} contains invalid code`), + {retain: false, qos: 0}, + expect.any(Function), + ); + expect(writeFileSyncSpy).not.toHaveBeenCalledWith(extensionFilePath, extensionCode, 'utf8'); + }); + + it('returns error on invalid removal', async () => { + const converterName = 'invalid.js'; + const converterFilePath = path.join(mockBasePath, converterName); + + await controller.start(); + await flushPromises(); + mocksClear.forEach((m) => m.mockClear()); + + mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/remove', stringify({name: converterName})); + await flushPromises(); + + expect(mockMQTT.publish).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/response/extension/remove', + stringify({data: {}, status: 'error', error: `${converterName} (${converterFilePath}) doesn't exists`}), + {retain: false, qos: 0}, + expect.any(Function), + ); + expect(rmSyncSpy).not.toHaveBeenCalledWith(converterFilePath, {force: true}); + }); +}); From 2f8d45f9fd2604fb4a27691aa98dbf5ca99ef919 Mon Sep 17 00:00:00 2001 From: Nerivec <62446222+Nerivec@users.noreply.github.com> Date: Thu, 14 Nov 2024 13:23:21 +0100 Subject: [PATCH 17/26] Fix use of ext conv in network map tests. --- test/extensions/networkMap.test.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/extensions/networkMap.test.ts b/test/extensions/networkMap.test.ts index 86d6eb25e6..d03794c3a4 100644 --- a/test/extensions/networkMap.test.ts +++ b/test/extensions/networkMap.test.ts @@ -102,8 +102,11 @@ describe('Extension: NetworkMap', () => { data.writeDefaultConfiguration(); settings.reRead(); data.writeEmptyState(); - fs.copyFileSync(path.join(__dirname, '..', 'assets', 'mock-external-converter.js'), path.join(data.mockDir, 'mock-external-converter.js')); - settings.set(['external_converters'], ['mock-external-converter.js']); + fs.mkdirSync(path.join(data.mockDir, 'external_converters')); + fs.copyFileSync( + path.join(__dirname, '..', 'assets', 'external_converters', 'mock-external-converter.js'), + path.join(data.mockDir, 'external_converters', 'mock-external-converter.js'), + ); controller = new Controller(jest.fn(), jest.fn()); await controller.start(); }); @@ -120,6 +123,7 @@ describe('Extension: NetworkMap', () => { afterAll(async () => { mockSleep.restore(); + fs.rmSync(path.join(data.mockDir, 'external_converters'), {recursive: true}); jest.useRealTimers(); }); From 68272add08b2f948041f92c727317b232557a5e5 Mon Sep 17 00:00:00 2001 From: Nerivec <62446222+Nerivec@users.noreply.github.com> Date: Thu, 14 Nov 2024 13:28:46 +0100 Subject: [PATCH 18/26] More coverage. --- lib/extension/externalJS.ts | 5 ++-- test/extensions/externalConverters.test.ts | 32 ++++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/lib/extension/externalJS.ts b/lib/extension/externalJS.ts index 4b997269f3..599adc8a06 100644 --- a/lib/extension/externalJS.ts +++ b/lib/extension/externalJS.ts @@ -65,6 +65,7 @@ export default abstract class ExternalJSExtension extends Extension { } for (const fileName of fs.readdirSync(this.basePath)) { + /* istanbul ignore else */ if (fileName.endsWith('.js')) { yield {name: fileName, code: this.getFileCode(fileName)}; } @@ -149,8 +150,8 @@ export default abstract class ExternalJSExtension extends Extension { ); } - private loadModuleFromText(moduleCode: string, name?: string): M { - const moduleFakePath = path.join(__dirname, '..', '..', 'data', 'extension', name || 'externally-loaded.js'); + private loadModuleFromText(moduleCode: string, name: string): M { + const moduleFakePath = path.join(__dirname, '..', '..', 'data', 'extension', name); const sandbox: Context = { require: require, module: {}, diff --git a/test/extensions/externalConverters.test.ts b/test/extensions/externalConverters.test.ts index f54603862e..fcd2481558 100644 --- a/test/extensions/externalConverters.test.ts +++ b/test/extensions/externalConverters.test.ts @@ -254,4 +254,36 @@ describe('Extension: ExternalConverters', () => { ); 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); + + await controller.start(); + await flushPromises(); + mocksClear.forEach((m) => m.mockClear()); + + //-- SAVE + mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/save', stringify({name: converterName, code: converterCode})); + await flushPromises(); + + const errorMsg = `Failed to remove definition`; + + mockZHCRemoveDefinition.mockImplementationOnce(() => { + throw new Error(errorMsg); + }); + + //-- REMOVE + mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/remove', stringify({name: converterName})); + await flushPromises(); + + expect(mockMQTT.publish).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/response/converter/remove', + stringify({data: {}, status: 'error', error: errorMsg}), + {retain: false, qos: 0}, + expect.any(Function), + ); + expect(rmSyncSpy).not.toHaveBeenCalledWith(converterFilePath, {force: true}); + }); }); From 6e025203e2703e8bccdb49d11f061bba1cf25a38 Mon Sep 17 00:00:00 2001 From: Nerivec <62446222+Nerivec@users.noreply.github.com> Date: Thu, 14 Nov 2024 14:17:26 +0100 Subject: [PATCH 19/26] Dont mock zhc for basics, tests actual live loading --- test/extensions/externalConverters.test.ts | 153 ++++++++++++--------- 1 file changed, 91 insertions(+), 62 deletions(-) diff --git a/test/extensions/externalConverters.test.ts b/test/extensions/externalConverters.test.ts index fcd2481558..44706e8a8e 100644 --- a/test/extensions/externalConverters.test.ts +++ b/test/extensions/externalConverters.test.ts @@ -4,6 +4,8 @@ import {mockMQTT, events as mockMQTTEvents} from '../mocks/mqtt'; import {flushPromises} from '../mocks/utils'; import {devices, mockController as mockZHController, returnDevices} from '../mocks/zigbeeHerdsman'; +import type Device from '../../lib/model/device'; + import fs from 'fs'; import path from 'path'; @@ -16,12 +18,8 @@ import * as settings from '../../lib/util/settings'; const BASE_DIR = 'external_converters'; -const mockZHCAddDefinition = jest.fn(); -const mockZHCRemoveDefinition = jest.fn(); -// @ts-expect-error mock -zhc.addDefinition = mockZHCAddDefinition; -// @ts-expect-error mock -zhc.removeDefinition = mockZHCRemoveDefinition; +// @ts-expect-error TODO: remove, tmp until implemented +zhc.removeDefinition = jest.fn(); describe('Extension: ExternalConverters', () => { const mockBasePath = path.join(data.mockDir, BASE_DIR); @@ -33,6 +31,9 @@ describe('Extension: ExternalConverters', () => { const rmSyncSpy = jest.spyOn(fs, 'rmSync'); const writeFileSyncSpy = jest.spyOn(fs, 'writeFileSync'); + const zhcAddDefinitionSpy = jest.spyOn(zhc, 'addDefinition'); + const zhcRemoveDefinitionSpy = jest.spyOn(zhc, 'removeDefinition'); + const mocksClear = [ mockMQTT.endAsync, mockMQTT.publishAsync, @@ -40,13 +41,13 @@ describe('Extension: ExternalConverters', () => { mockLogger.error, mockZHController.stop, devices.bulb.save, - mockZHCAddDefinition, - mockZHCRemoveDefinition, existsSyncSpy, readdirSyncSpy, mkdirSyncSpy, rmSyncSpy, writeFileSyncSpy, + zhcAddDefinitionSpy, + zhcRemoveDefinitionSpy, ]; const useAssets = (): void => { @@ -57,6 +58,11 @@ describe('Extension: ExternalConverters', () => { return fs.readFileSync(path.join(__dirname, '..', 'assets', BASE_DIR, fileName), 'utf8'); }; + const getZ2MDevice = (zhDevice: unknown): Device => { + // @ts-expect-error private + return controller.zigbee.resolveEntity(zhDevice)! as Device; + }; + beforeAll(async () => { jest.useFakeTimers(); }); @@ -70,7 +76,7 @@ describe('Extension: ExternalConverters', () => { data.writeDefaultConfiguration(); data.writeDefaultState(); settings.reRead(); - returnDevices.splice(0); + returnDevices.push(devices.external_converter_device.ieeeAddr); controller = new Controller(jest.fn(), jest.fn()); }); @@ -94,6 +100,12 @@ describe('Extension: ExternalConverters', () => { await controller.start(); await flushPromises(); + expect(getZ2MDevice(devices.external_converter_device)!.definition).toMatchObject({ + description: 'external', + model: 'external_converter_device', + vendor: 'external', + zigbeeModel: ['external_converter_device'], + }); expect(mockMQTT.publish).toHaveBeenCalledWith( 'zigbee2mqtt/bridge/converters', stringify([ @@ -103,37 +115,37 @@ describe('Extension: ExternalConverters', () => { {retain: true, qos: 0}, expect.any(Function), ); - expect(mockZHCRemoveDefinition).toHaveBeenCalledTimes(3); - expect(mockZHCAddDefinition).toHaveBeenNthCalledWith(1, { - mock: 1, - model: 'external_converters_device_1', - zigbeeModel: ['external_converter_device_1'], - vendor: 'external_1', - description: 'external_1', - fromZigbee: [], - toZigbee: [], - exposes: [], - }); - expect(mockZHCAddDefinition).toHaveBeenNthCalledWith(2, { - mock: 2, - model: 'external_converters_device_2', - zigbeeModel: ['external_converter_device_2'], - vendor: 'external_2', - description: 'external_2', - fromZigbee: [], - toZigbee: [], - exposes: [], - }); - expect(mockZHCAddDefinition).toHaveBeenNthCalledWith(3, { - mock: true, - zigbeeModel: ['external_converter_device'], - vendor: 'external', - model: 'external_converter_device', - description: 'external', - fromZigbee: [], - toZigbee: [], - exposes: [], - }); + expect(zhcRemoveDefinitionSpy).toHaveBeenCalledTimes(3); + expect(zhcAddDefinitionSpy).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + mock: 1, + model: 'external_converters_device_1', + zigbeeModel: ['external_converter_device_1'], + vendor: 'external_1', + description: 'external_1', + }), + ); + expect(zhcAddDefinitionSpy).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + mock: 2, + model: 'external_converters_device_2', + zigbeeModel: ['external_converter_device_2'], + vendor: 'external_2', + description: 'external_2', + }), + ); + expect(zhcAddDefinitionSpy).toHaveBeenNthCalledWith( + 3, + expect.objectContaining({ + mock: true, + zigbeeModel: ['external_converter_device'], + vendor: 'external', + model: 'external_converter_device', + description: 'external', + }), + ); }); it('saves and removes from MQTT', async () => { @@ -145,23 +157,35 @@ describe('Extension: ExternalConverters', () => { await flushPromises(); mocksClear.forEach((m) => m.mockClear()); + expect(getZ2MDevice(devices.external_converter_device)!.definition).toMatchObject({ + description: 'Automatically generated definition', + model: 'external_converter_device', + vendor: '', + zigbeeModel: ['external_converter_device'], + }); + //-- SAVE mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/save', stringify({name: converterName, code: converterCode})); await flushPromises(); - expect(mkdirSyncSpy).toHaveBeenCalledWith(mockBasePath, {recursive: true}); - expect(writeFileSyncSpy).toHaveBeenCalledWith(converterFilePath, converterCode, 'utf8'); - expect(mockZHCRemoveDefinition).toHaveBeenCalledTimes(1); - expect(mockZHCAddDefinition).toHaveBeenCalledWith({ - mock: true, - zigbeeModel: ['external_converter_device'], - vendor: 'external', - model: 'external_converter_device', + expect(getZ2MDevice(devices.external_converter_device)!.definition).toMatchObject({ description: 'external', - fromZigbee: [], - toZigbee: [], - exposes: [], + model: 'external_converter_device', + vendor: 'external', + zigbeeModel: ['external_converter_device'], }); + expect(mkdirSyncSpy).toHaveBeenCalledWith(mockBasePath, {recursive: true}); + expect(writeFileSyncSpy).toHaveBeenCalledWith(converterFilePath, converterCode, 'utf8'); + expect(zhcRemoveDefinitionSpy).toHaveBeenCalledTimes(1); + expect(zhcAddDefinitionSpy).toHaveBeenCalledWith( + expect.objectContaining({ + mock: true, + zigbeeModel: ['external_converter_device'], + vendor: 'external', + model: 'external_converter_device', + description: 'external', + }), + ); expect(mockMQTT.publish).toHaveBeenCalledWith( 'zigbee2mqtt/bridge/converters', stringify([{name: converterName, code: converterCode}]), @@ -173,17 +197,22 @@ describe('Extension: ExternalConverters', () => { mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/remove', stringify({name: converterName})); await flushPromises(); - expect(rmSyncSpy).toHaveBeenCalledWith(converterFilePath, {force: true}); - expect(mockZHCRemoveDefinition).toHaveBeenCalledWith({ - mock: true, - zigbeeModel: ['external_converter_device'], - vendor: 'external', + expect(getZ2MDevice(devices.external_converter_device)!.definition).toMatchObject({ + description: 'Automatically generated definition', model: 'external_converter_device', - description: 'external', - fromZigbee: [], - toZigbee: [], - exposes: [], + vendor: '', + zigbeeModel: ['external_converter_device'], }); + expect(rmSyncSpy).toHaveBeenCalledWith(converterFilePath, {force: true}); + expect(zhcRemoveDefinitionSpy).toHaveBeenCalledWith( + expect.objectContaining({ + mock: true, + zigbeeModel: ['external_converter_device'], + vendor: 'external', + model: 'external_converter_device', + description: 'external', + }), + ); expect(mockMQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bridge/converters', stringify([]), {retain: true, qos: 0}, expect.any(Function)); }); @@ -239,7 +268,7 @@ describe('Extension: ExternalConverters', () => { const errorMsg = `Invalid definition`; - mockZHCAddDefinition.mockImplementationOnce(() => { + zhcAddDefinitionSpy.mockImplementationOnce(() => { throw new Error(errorMsg); }); @@ -270,7 +299,7 @@ describe('Extension: ExternalConverters', () => { const errorMsg = `Failed to remove definition`; - mockZHCRemoveDefinition.mockImplementationOnce(() => { + zhcRemoveDefinitionSpy.mockImplementationOnce(() => { throw new Error(errorMsg); }); From 1f483a42e538eed3d0af22a05c807353509466bd Mon Sep 17 00:00:00 2001 From: Nerivec <62446222+Nerivec@users.noreply.github.com> Date: Sun, 17 Nov 2024 20:22:35 +0100 Subject: [PATCH 20/26] Update --- lib/extension/externalConverters.ts | 38 +++++++++++--------- lib/model/device.ts | 2 +- lib/zigbee.ts | 10 ++++-- test/extensions/externalConverters.test.ts | 40 ++++++++++------------ 4 files changed, 47 insertions(+), 43 deletions(-) diff --git a/lib/extension/externalConverters.ts b/lib/extension/externalConverters.ts index a958c3d33c..79eefe0f69 100644 --- a/lib/extension/externalConverters.ts +++ b/lib/extension/externalConverters.ts @@ -1,4 +1,4 @@ -import {addDefinition, removeDefinition} from 'zigbee-herdsman-converters'; +import {addDefinition, removeExternalDefinitions} from 'zigbee-herdsman-converters'; import logger from '../util/logger'; import ExternalJSExtension from './externalJS'; @@ -30,29 +30,33 @@ export default class ExternalConverters extends ExternalJSExtension { - for (const definition of this.getDefinitions(module)) { - // TODO: implement in ZHC - removeDefinition(definition); - } + removeExternalDefinitions(name); + + await this.zigbee.resolveDevicesDefinitions(true); } protected async loadJS(name: string, module: ModuleExports): Promise { - for (const definition of this.getDefinitions(module)) { - try { - // TODO: `updateDefinition` in ZHC instead? (add if not exist, replace if exist) - removeDefinition(definition); + try { + removeExternalDefinitions(name); + + for (const definition of this.getDefinitions(module)) { + definition.externalConverterName = name; + addDefinition(definition); logger.info(`Loaded external converter '${name}'.`); - } catch (error) { - logger.error(`Failed to load external converter '${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`, - ); - - throw error; } + + await this.zigbee.resolveDevicesDefinitions(true); + } catch (error) { + logger.error(`Failed to load external converter '${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`, + ); + + throw error; } } diff --git a/lib/model/device.ts b/lib/model/device.ts index 0032eea5b6..51b45728d8 100644 --- a/lib/model/device.ts +++ b/lib/model/device.ts @@ -48,7 +48,7 @@ export default class Device { } } - async resolveDefinition(ignoreCache = false): Promise { + async resolveDefinition(ignoreCache: boolean = false): Promise { if (!this.zh.interviewing && (!this.definition || this._definitionModelID !== this.zh.modelID || ignoreCache)) { this.definition = await zhc.findByDevice(this.zh, true); this._definitionModelID = this.zh.modelID; diff --git a/lib/zigbee.ts b/lib/zigbee.ts index 8a7e5e2b52..18a173ac3a 100644 --- a/lib/zigbee.ts +++ b/lib/zigbee.ts @@ -72,9 +72,7 @@ export default class Zigbee { throw error; } - for (const device of this.devicesIterator(utils.deviceNotCoordinator)) { - await device.resolveDefinition(); - } + await this.resolveDevicesDefinitions(); this.herdsman.on('adapterDisconnected', () => this.eventBus.emitAdapterDisconnected()); this.herdsman.on('lastSeenChanged', (data: ZHEvents.LastSeenChangedPayload) => { @@ -239,6 +237,12 @@ export default class Zigbee { await this.herdsman.permitJoin(time, device?.zh); } + async resolveDevicesDefinitions(ignoreCache: boolean = false): Promise { + for (const device of this.devicesIterator(utils.deviceNotCoordinator)) { + await device.resolveDefinition(ignoreCache); + } + } + @bind private resolveDevice(ieeeAddr: string): Device | undefined { if (!this.deviceLookup[ieeeAddr]) { const device = this.herdsman.getDeviceByIeeeAddr(ieeeAddr); diff --git a/test/extensions/externalConverters.test.ts b/test/extensions/externalConverters.test.ts index 44706e8a8e..75bc7bf593 100644 --- a/test/extensions/externalConverters.test.ts +++ b/test/extensions/externalConverters.test.ts @@ -18,9 +18,6 @@ import * as settings from '../../lib/util/settings'; const BASE_DIR = 'external_converters'; -// @ts-expect-error TODO: remove, tmp until implemented -zhc.removeDefinition = jest.fn(); - describe('Extension: ExternalConverters', () => { const mockBasePath = path.join(data.mockDir, BASE_DIR); let controller: Controller; @@ -32,7 +29,7 @@ describe('Extension: ExternalConverters', () => { const writeFileSyncSpy = jest.spyOn(fs, 'writeFileSync'); const zhcAddDefinitionSpy = jest.spyOn(zhc, 'addDefinition'); - const zhcRemoveDefinitionSpy = jest.spyOn(zhc, 'removeDefinition'); + const zhcRemoveExternalDefinitionsSpy = jest.spyOn(zhc, 'removeExternalDefinitions'); const mocksClear = [ mockMQTT.endAsync, @@ -47,7 +44,7 @@ describe('Extension: ExternalConverters', () => { rmSyncSpy, writeFileSyncSpy, zhcAddDefinitionSpy, - zhcRemoveDefinitionSpy, + zhcRemoveExternalDefinitionsSpy, ]; const useAssets = (): void => { @@ -72,6 +69,7 @@ describe('Extension: ExternalConverters', () => { }); beforeEach(async () => { + zhc.removeExternalDefinitions(); // remove all external converters mocksClear.forEach((m) => m.mockClear()); data.writeDefaultConfiguration(); data.writeDefaultState(); @@ -81,8 +79,10 @@ describe('Extension: ExternalConverters', () => { controller = new Controller(jest.fn(), jest.fn()); }); - afterEach(() => { + afterEach(async () => { fs.rmSync(mockBasePath, {recursive: true, force: true}); + + await controller?.stop(); }); it('loads nothing from folder', async () => { @@ -100,7 +100,7 @@ describe('Extension: ExternalConverters', () => { await controller.start(); await flushPromises(); - expect(getZ2MDevice(devices.external_converter_device)!.definition).toMatchObject({ + expect(getZ2MDevice(devices.external_converter_device).definition).toMatchObject({ description: 'external', model: 'external_converter_device', vendor: 'external', @@ -115,7 +115,9 @@ describe('Extension: ExternalConverters', () => { {retain: true, qos: 0}, expect.any(Function), ); - expect(zhcRemoveDefinitionSpy).toHaveBeenCalledTimes(3); + expect(zhcRemoveExternalDefinitionsSpy).toHaveBeenCalledTimes(2); + expect(zhcRemoveExternalDefinitionsSpy).toHaveBeenNthCalledWith(1, 'mock-external-converter-multiple.js'); + expect(zhcRemoveExternalDefinitionsSpy).toHaveBeenNthCalledWith(2, 'mock-external-converter.js'); expect(zhcAddDefinitionSpy).toHaveBeenNthCalledWith( 1, expect.objectContaining({ @@ -157,7 +159,7 @@ describe('Extension: ExternalConverters', () => { await flushPromises(); mocksClear.forEach((m) => m.mockClear()); - expect(getZ2MDevice(devices.external_converter_device)!.definition).toMatchObject({ + expect(getZ2MDevice(devices.external_converter_device).definition).toMatchObject({ description: 'Automatically generated definition', model: 'external_converter_device', vendor: '', @@ -168,7 +170,7 @@ describe('Extension: ExternalConverters', () => { mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/save', stringify({name: converterName, code: converterCode})); await flushPromises(); - expect(getZ2MDevice(devices.external_converter_device)!.definition).toMatchObject({ + expect(getZ2MDevice(devices.external_converter_device).definition).toMatchObject({ description: 'external', model: 'external_converter_device', vendor: 'external', @@ -176,7 +178,8 @@ describe('Extension: ExternalConverters', () => { }); expect(mkdirSyncSpy).toHaveBeenCalledWith(mockBasePath, {recursive: true}); expect(writeFileSyncSpy).toHaveBeenCalledWith(converterFilePath, converterCode, 'utf8'); - expect(zhcRemoveDefinitionSpy).toHaveBeenCalledTimes(1); + expect(zhcRemoveExternalDefinitionsSpy).toHaveBeenCalledTimes(1); + expect(zhcRemoveExternalDefinitionsSpy).toHaveBeenNthCalledWith(1, converterName); expect(zhcAddDefinitionSpy).toHaveBeenCalledWith( expect.objectContaining({ mock: true, @@ -197,22 +200,15 @@ describe('Extension: ExternalConverters', () => { mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/remove', stringify({name: converterName})); await flushPromises(); - expect(getZ2MDevice(devices.external_converter_device)!.definition).toMatchObject({ + 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(zhcRemoveDefinitionSpy).toHaveBeenCalledWith( - expect.objectContaining({ - mock: true, - zigbeeModel: ['external_converter_device'], - vendor: 'external', - model: 'external_converter_device', - description: 'external', - }), - ); + expect(zhcRemoveExternalDefinitionsSpy).toHaveBeenCalledTimes(2); + expect(zhcRemoveExternalDefinitionsSpy).toHaveBeenNthCalledWith(2, converterName); expect(mockMQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bridge/converters', stringify([]), {retain: true, qos: 0}, expect.any(Function)); }); @@ -299,7 +295,7 @@ describe('Extension: ExternalConverters', () => { const errorMsg = `Failed to remove definition`; - zhcRemoveDefinitionSpy.mockImplementationOnce(() => { + zhcRemoveExternalDefinitionsSpy.mockImplementationOnce(() => { throw new Error(errorMsg); }); From 13f0b2c5142864f053b19875d7719d2a2f56f059 Mon Sep 17 00:00:00 2001 From: Nerivec <62446222+Nerivec@users.noreply.github.com> Date: Tue, 19 Nov 2024 23:31:42 +0100 Subject: [PATCH 21/26] Fix rebase --- test/extensions/externalConverters.test.ts | 22 +++++------- test/extensions/externalExtensions.test.ts | 41 ++++++++-------------- 2 files changed, 23 insertions(+), 40 deletions(-) diff --git a/test/extensions/externalConverters.test.ts b/test/extensions/externalConverters.test.ts index 75bc7bf593..0ed1afaea1 100644 --- a/test/extensions/externalConverters.test.ts +++ b/test/extensions/externalConverters.test.ts @@ -91,7 +91,7 @@ describe('Extension: ExternalConverters', () => { expect(existsSyncSpy).toHaveBeenCalledWith(mockBasePath); expect(readdirSyncSpy).not.toHaveBeenCalledWith(mockBasePath); - expect(mockMQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bridge/converters', stringify([]), {retain: true, qos: 0}, expect.any(Function)); + expect(mockMQTT.publishAsync).toHaveBeenCalledWith('zigbee2mqtt/bridge/converters', stringify([]), {retain: true, qos: 0}); }); it('loads from folder', async () => { @@ -106,14 +106,13 @@ describe('Extension: ExternalConverters', () => { vendor: 'external', zigbeeModel: ['external_converter_device'], }); - expect(mockMQTT.publish).toHaveBeenCalledWith( + expect(mockMQTT.publishAsync).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')}, ]), {retain: true, qos: 0}, - expect.any(Function), ); expect(zhcRemoveExternalDefinitionsSpy).toHaveBeenCalledTimes(2); expect(zhcRemoveExternalDefinitionsSpy).toHaveBeenNthCalledWith(1, 'mock-external-converter-multiple.js'); @@ -189,11 +188,10 @@ describe('Extension: ExternalConverters', () => { description: 'external', }), ); - expect(mockMQTT.publish).toHaveBeenCalledWith( + expect(mockMQTT.publishAsync).toHaveBeenCalledWith( 'zigbee2mqtt/bridge/converters', stringify([{name: converterName, code: converterCode}]), {retain: true, qos: 0}, - expect.any(Function), ); //-- REMOVE @@ -209,7 +207,7 @@ describe('Extension: ExternalConverters', () => { expect(rmSyncSpy).toHaveBeenCalledWith(converterFilePath, {force: true}); expect(zhcRemoveExternalDefinitionsSpy).toHaveBeenCalledTimes(2); expect(zhcRemoveExternalDefinitionsSpy).toHaveBeenNthCalledWith(2, converterName); - expect(mockMQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bridge/converters', stringify([]), {retain: true, qos: 0}, expect.any(Function)); + expect(mockMQTT.publishAsync).toHaveBeenCalledWith('zigbee2mqtt/bridge/converters', stringify([]), {retain: true, qos: 0}, expect.any(Function)); }); it('returns error on invalid code', async () => { @@ -224,11 +222,10 @@ describe('Extension: ExternalConverters', () => { mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/save', stringify({name: converterName, code: converterCode})); await flushPromises(); - expect(mockMQTT.publish).toHaveBeenCalledWith( + expect(mockMQTT.publishAsync).toHaveBeenCalledWith( 'zigbee2mqtt/bridge/response/converter/save', expect.stringContaining(`"error":"foo.js contains invalid code`), {retain: false, qos: 0}, - expect.any(Function), ); expect(writeFileSyncSpy).not.toHaveBeenCalledWith(converterFilePath, converterCode, 'utf8'); }); @@ -244,11 +241,10 @@ describe('Extension: ExternalConverters', () => { mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/remove', stringify({name: converterName})); await flushPromises(); - expect(mockMQTT.publish).toHaveBeenCalledWith( + expect(mockMQTT.publishAsync).toHaveBeenCalledWith( 'zigbee2mqtt/bridge/response/converter/remove', stringify({data: {}, status: 'error', error: `${converterName} (${converterFilePath}) doesn't exists`}), {retain: false, qos: 0}, - expect.any(Function), ); expect(rmSyncSpy).not.toHaveBeenCalledWith(converterFilePath, {force: true}); }); @@ -271,11 +267,10 @@ describe('Extension: ExternalConverters', () => { mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/save', stringify({name: converterName, code: converterCode})); await flushPromises(); - expect(mockMQTT.publish).toHaveBeenCalledWith( + expect(mockMQTT.publishAsync).toHaveBeenCalledWith( 'zigbee2mqtt/bridge/response/converter/save', expect.stringContaining(errorMsg), {retain: false, qos: 0}, - expect.any(Function), ); expect(writeFileSyncSpy).not.toHaveBeenCalledWith(converterFilePath, converterCode, 'utf8'); }); @@ -303,11 +298,10 @@ describe('Extension: ExternalConverters', () => { mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/remove', stringify({name: converterName})); await flushPromises(); - expect(mockMQTT.publish).toHaveBeenCalledWith( + expect(mockMQTT.publishAsync).toHaveBeenCalledWith( 'zigbee2mqtt/bridge/response/converter/remove', stringify({data: {}, status: 'error', error: errorMsg}), {retain: false, qos: 0}, - expect.any(Function), ); expect(rmSyncSpy).not.toHaveBeenCalledWith(converterFilePath, {force: true}); }); diff --git a/test/extensions/externalExtensions.test.ts b/test/extensions/externalExtensions.test.ts index ee5d654fd9..de33ed28da 100644 --- a/test/extensions/externalExtensions.test.ts +++ b/test/extensions/externalExtensions.test.ts @@ -25,8 +25,8 @@ describe('Extension: ExternalExtensions', () => { const writeFileSyncSpy = jest.spyOn(fs, 'writeFileSync'); const mocksClear = [ - mockMQTT.end, - mockMQTT.publish, + mockMQTT.endAsync, + mockMQTT.publishAsync, mockLogger.debug, mockLogger.error, mockZHController.stop, @@ -74,7 +74,7 @@ describe('Extension: ExternalExtensions', () => { expect(existsSyncSpy).toHaveBeenCalledWith(mockBasePath); expect(readdirSyncSpy).not.toHaveBeenCalledWith(mockBasePath); - expect(mockMQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bridge/extensions', stringify([]), {retain: true, qos: 0}, expect.any(Function)); + expect(mockMQTT.publishAsync).toHaveBeenCalledWith('zigbee2mqtt/bridge/extensions', stringify([]), {retain: true, qos: 0}); }); it('loads from folder', async () => { @@ -83,38 +83,33 @@ describe('Extension: ExternalExtensions', () => { await controller.start(); await flushPromises(); - expect(mockMQTT.publish).toHaveBeenCalledWith( + expect(mockMQTT.publishAsync).toHaveBeenCalledWith( 'zigbee2mqtt/example2/extension', 'call2 from constructor', {retain: false, qos: 0}, - expect.any(Function), ); - expect(mockMQTT.publish).toHaveBeenCalledWith( + expect(mockMQTT.publishAsync).toHaveBeenCalledWith( 'zigbee2mqtt/example2/extension', 'call2 from start', {retain: false, qos: 0}, - expect.any(Function), ); - expect(mockMQTT.publish).toHaveBeenCalledWith( + expect(mockMQTT.publishAsync).toHaveBeenCalledWith( 'zigbee2mqtt/example/extension', 'call from constructor', {retain: false, qos: 0}, - expect.any(Function), ); - expect(mockMQTT.publish).toHaveBeenCalledWith( + expect(mockMQTT.publishAsync).toHaveBeenCalledWith( 'zigbee2mqtt/example/extension', 'call from start', {retain: false, qos: 0}, - expect.any(Function), ); - expect(mockMQTT.publish).toHaveBeenCalledWith( + expect(mockMQTT.publishAsync).toHaveBeenCalledWith( 'zigbee2mqtt/bridge/extensions', stringify([ {name: 'example2Extension.js', code: getFileCode('example2Extension.js')}, {name: 'exampleExtension.js', code: getFileCode('exampleExtension.js')}, ]), {retain: true, qos: 0}, - expect.any(Function), ); }); @@ -133,23 +128,20 @@ describe('Extension: ExternalExtensions', () => { expect(mkdirSyncSpy).toHaveBeenCalledWith(mockBasePath, {recursive: true}); expect(writeFileSyncSpy).toHaveBeenCalledWith(extensionFilePath, extensionCode, 'utf8'); - expect(mockMQTT.publish).toHaveBeenCalledWith( + expect(mockMQTT.publishAsync).toHaveBeenCalledWith( 'zigbee2mqtt/example/extension', 'call from constructor', {retain: false, qos: 0}, - expect.any(Function), ); - expect(mockMQTT.publish).toHaveBeenCalledWith( + expect(mockMQTT.publishAsync).toHaveBeenCalledWith( 'zigbee2mqtt/example/extension', 'call from start', {retain: false, qos: 0}, - expect.any(Function), ); - expect(mockMQTT.publish).toHaveBeenCalledWith( + expect(mockMQTT.publishAsync).toHaveBeenCalledWith( 'zigbee2mqtt/bridge/extensions', stringify([{name: extensionName, code: extensionCode}]), {retain: true, qos: 0}, - expect.any(Function), ); //-- REMOVE @@ -157,13 +149,12 @@ describe('Extension: ExternalExtensions', () => { await flushPromises(); expect(rmSyncSpy).toHaveBeenCalledWith(extensionFilePath, {force: true}); - expect(mockMQTT.publish).toHaveBeenCalledWith( + expect(mockMQTT.publishAsync).toHaveBeenCalledWith( 'zigbee2mqtt/example/extension', 'call from stop', {retain: false, qos: 0}, - expect.any(Function), ); - expect(mockMQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bridge/extensions', stringify([]), {retain: true, qos: 0}, expect.any(Function)); + expect(mockMQTT.publishAsync).toHaveBeenCalledWith('zigbee2mqtt/bridge/extensions', stringify([]), {retain: true, qos: 0}, expect.any(Function)); }); it('returns error on invalid code', async () => { @@ -178,11 +169,10 @@ describe('Extension: ExternalExtensions', () => { mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/save', stringify({name: extensionName, code: extensionCode})); await flushPromises(); - expect(mockMQTT.publish).toHaveBeenCalledWith( + expect(mockMQTT.publishAsync).toHaveBeenCalledWith( 'zigbee2mqtt/bridge/response/extension/save', expect.stringContaining(`"error":"${extensionName} contains invalid code`), {retain: false, qos: 0}, - expect.any(Function), ); expect(writeFileSyncSpy).not.toHaveBeenCalledWith(extensionFilePath, extensionCode, 'utf8'); }); @@ -198,11 +188,10 @@ describe('Extension: ExternalExtensions', () => { mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/remove', stringify({name: converterName})); await flushPromises(); - expect(mockMQTT.publish).toHaveBeenCalledWith( + expect(mockMQTT.publishAsync).toHaveBeenCalledWith( 'zigbee2mqtt/bridge/response/extension/remove', stringify({data: {}, status: 'error', error: `${converterName} (${converterFilePath}) doesn't exists`}), {retain: false, qos: 0}, - expect.any(Function), ); expect(rmSyncSpy).not.toHaveBeenCalledWith(converterFilePath, {force: true}); }); From 3e4b327022dbbc487a7eaaecdb0116c4ee3c5a03 Mon Sep 17 00:00:00 2001 From: Nerivec <62446222+Nerivec@users.noreply.github.com> Date: Tue, 19 Nov 2024 23:36:27 +0100 Subject: [PATCH 22/26] Fix --- test/extensions/externalConverters.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/extensions/externalConverters.test.ts b/test/extensions/externalConverters.test.ts index 0ed1afaea1..bb0f2f186f 100644 --- a/test/extensions/externalConverters.test.ts +++ b/test/extensions/externalConverters.test.ts @@ -207,7 +207,7 @@ describe('Extension: ExternalConverters', () => { expect(rmSyncSpy).toHaveBeenCalledWith(converterFilePath, {force: true}); expect(zhcRemoveExternalDefinitionsSpy).toHaveBeenCalledTimes(2); expect(zhcRemoveExternalDefinitionsSpy).toHaveBeenNthCalledWith(2, converterName); - expect(mockMQTT.publishAsync).toHaveBeenCalledWith('zigbee2mqtt/bridge/converters', stringify([]), {retain: true, qos: 0}, expect.any(Function)); + expect(mockMQTT.publishAsync).toHaveBeenCalledWith('zigbee2mqtt/bridge/converters', stringify([]), {retain: true, qos: 0}); }); it('returns error on invalid code', async () => { From 2827eac01a9acc63321b48c280e1aec7591784a1 Mon Sep 17 00:00:00 2001 From: Koen Kanters Date: Thu, 21 Nov 2024 22:24:09 +0100 Subject: [PATCH 23/26] Bump zhc --- package.json | 2 +- pnpm-lock.yaml | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/package.json b/package.json index c9f3986a5a..9d9363a8d6 100644 --- a/package.json +++ b/package.json @@ -61,7 +61,7 @@ "winston-transport": "^4.8.0", "ws": "^8.18.0", "zigbee-herdsman": "3.0.0-pre.1", - "zigbee-herdsman-converters": "21.0.0-pre.1", + "zigbee-herdsman-converters": "21.0.0-pre.2", "zigbee2mqtt-frontend": "0.7.4" }, "devDependencies": { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 5b0c29511c..629b329fca 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -84,8 +84,8 @@ importers: specifier: 3.0.0-pre.1 version: 3.0.0-pre.1 zigbee-herdsman-converters: - specifier: 21.0.0-pre.1 - version: 21.0.0-pre.1 + specifier: 21.0.0-pre.2 + version: 21.0.0-pre.2 zigbee2mqtt-frontend: specifier: 0.7.4 version: 0.7.4 @@ -2816,8 +2816,8 @@ packages: resolution: {integrity: sha512-rVksvsnNCdJ/ohGc6xgPwyN8eheCxsiLM8mxuE/t/mOVqJewPuO1miLpTHQiRgTKCLexL4MeAFVagts7HmNZ2Q==} engines: {node: '>=10'} - zigbee-herdsman-converters@21.0.0-pre.1: - resolution: {integrity: sha512-ALNeNua3OiQtSGkBn8W2G2XfWrytZjV5VbzwyeIJPYzEpMqMIvBhbXMyF/l6AP88b68dYFVKqjJMuTxXbvCETQ==} + zigbee-herdsman-converters@21.0.0-pre.2: + resolution: {integrity: sha512-nT9yxeCn1N6MecOgSJ2/u6R6Yj55oS47seRTHnpV+xk0yR2C6a2iC1bxSxxAj4iVLQDu6Gcqjgvpfs/1ds8Drw==} zigbee-herdsman@3.0.0-pre.1: resolution: {integrity: sha512-HRonSIS39LwWo0EBA2YRE/E9o6g0tY8RT8TkjlxH6R4uwJwPrE2mIzhLMewqU3xWbmpS6YamsZ7jw/GY9U3byw==} @@ -5968,7 +5968,7 @@ snapshots: yocto-queue@0.1.0: {} - zigbee-herdsman-converters@21.0.0-pre.1: + zigbee-herdsman-converters@21.0.0-pre.2: dependencies: buffer-crc32: 1.0.0 iconv-lite: 0.6.3 From 690ef48d76420ca590b238f7a9a7e2b161d92d7d Mon Sep 17 00:00:00 2001 From: Koen Kanters Date: Thu, 21 Nov 2024 22:25:29 +0100 Subject: [PATCH 24/26] pretty --- test/extensions/externalConverters.test.ts | 18 ++++---- test/extensions/externalExtensions.test.ts | 51 +++++----------------- 2 files changed, 19 insertions(+), 50 deletions(-) diff --git a/test/extensions/externalConverters.test.ts b/test/extensions/externalConverters.test.ts index bb0f2f186f..f64fec9f84 100644 --- a/test/extensions/externalConverters.test.ts +++ b/test/extensions/externalConverters.test.ts @@ -188,11 +188,10 @@ describe('Extension: ExternalConverters', () => { description: 'external', }), ); - expect(mockMQTT.publishAsync).toHaveBeenCalledWith( - 'zigbee2mqtt/bridge/converters', - stringify([{name: converterName, code: converterCode}]), - {retain: true, qos: 0}, - ); + expect(mockMQTT.publishAsync).toHaveBeenCalledWith('zigbee2mqtt/bridge/converters', stringify([{name: converterName, code: converterCode}]), { + retain: true, + qos: 0, + }); //-- REMOVE mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/remove', stringify({name: converterName})); @@ -267,11 +266,10 @@ describe('Extension: ExternalConverters', () => { mockMQTTEvents.message('zigbee2mqtt/bridge/request/converter/save', stringify({name: converterName, code: converterCode})); await flushPromises(); - expect(mockMQTT.publishAsync).toHaveBeenCalledWith( - 'zigbee2mqtt/bridge/response/converter/save', - expect.stringContaining(errorMsg), - {retain: false, qos: 0}, - ); + expect(mockMQTT.publishAsync).toHaveBeenCalledWith('zigbee2mqtt/bridge/response/converter/save', expect.stringContaining(errorMsg), { + retain: false, + qos: 0, + }); expect(writeFileSyncSpy).not.toHaveBeenCalledWith(converterFilePath, converterCode, 'utf8'); }); diff --git a/test/extensions/externalExtensions.test.ts b/test/extensions/externalExtensions.test.ts index f8b2067579..747b6d29ce 100644 --- a/test/extensions/externalExtensions.test.ts +++ b/test/extensions/externalExtensions.test.ts @@ -83,26 +83,10 @@ describe('Extension: ExternalExtensions', () => { await controller.start(); await flushPromises(); - expect(mockMQTT.publishAsync).toHaveBeenCalledWith( - 'zigbee2mqtt/example2/extension', - 'call2 from constructor', - {retain: false, qos: 0}, - ); - expect(mockMQTT.publishAsync).toHaveBeenCalledWith( - 'zigbee2mqtt/example2/extension', - 'call2 from start', - {retain: false, qos: 0}, - ); - expect(mockMQTT.publishAsync).toHaveBeenCalledWith( - 'zigbee2mqtt/example/extension', - 'call from constructor', - {retain: false, qos: 0}, - ); - expect(mockMQTT.publishAsync).toHaveBeenCalledWith( - 'zigbee2mqtt/example/extension', - 'call from start', - {retain: false, qos: 0}, - ); + expect(mockMQTT.publishAsync).toHaveBeenCalledWith('zigbee2mqtt/example2/extension', 'call2 from constructor', {retain: false, qos: 0}); + expect(mockMQTT.publishAsync).toHaveBeenCalledWith('zigbee2mqtt/example2/extension', 'call2 from start', {retain: false, qos: 0}); + expect(mockMQTT.publishAsync).toHaveBeenCalledWith('zigbee2mqtt/example/extension', 'call from constructor', {retain: false, qos: 0}); + expect(mockMQTT.publishAsync).toHaveBeenCalledWith('zigbee2mqtt/example/extension', 'call from start', {retain: false, qos: 0}); expect(mockMQTT.publishAsync).toHaveBeenCalledWith( 'zigbee2mqtt/bridge/extensions', stringify([ @@ -128,32 +112,19 @@ describe('Extension: ExternalExtensions', () => { expect(mkdirSyncSpy).toHaveBeenCalledWith(mockBasePath, {recursive: true}); expect(writeFileSyncSpy).toHaveBeenCalledWith(extensionFilePath, extensionCode, 'utf8'); - expect(mockMQTT.publishAsync).toHaveBeenCalledWith( - 'zigbee2mqtt/example/extension', - 'call from constructor', - {retain: false, qos: 0}, - ); - expect(mockMQTT.publishAsync).toHaveBeenCalledWith( - 'zigbee2mqtt/example/extension', - 'call from start', - {retain: false, qos: 0}, - ); - expect(mockMQTT.publishAsync).toHaveBeenCalledWith( - 'zigbee2mqtt/bridge/extensions', - stringify([{name: extensionName, code: extensionCode}]), - {retain: true, qos: 0}, - ); + expect(mockMQTT.publishAsync).toHaveBeenCalledWith('zigbee2mqtt/example/extension', 'call from constructor', {retain: false, qos: 0}); + expect(mockMQTT.publishAsync).toHaveBeenCalledWith('zigbee2mqtt/example/extension', 'call from start', {retain: false, qos: 0}); + expect(mockMQTT.publishAsync).toHaveBeenCalledWith('zigbee2mqtt/bridge/extensions', stringify([{name: extensionName, code: extensionCode}]), { + retain: true, + qos: 0, + }); //-- REMOVE mockMQTTEvents.message('zigbee2mqtt/bridge/request/extension/remove', stringify({name: extensionName})); await flushPromises(); expect(rmSyncSpy).toHaveBeenCalledWith(extensionFilePath, {force: true}); - expect(mockMQTT.publishAsync).toHaveBeenCalledWith( - 'zigbee2mqtt/example/extension', - 'call from stop', - {retain: false, qos: 0}, - ); + expect(mockMQTT.publishAsync).toHaveBeenCalledWith('zigbee2mqtt/example/extension', 'call from stop', {retain: false, qos: 0}); expect(mockMQTT.publishAsync).toHaveBeenCalledWith('zigbee2mqtt/bridge/extensions', stringify([]), {retain: true, qos: 0}); }); From 77a5f11105e2686d28751a26cb287f438b7b2757 Mon Sep 17 00:00:00 2001 From: Koen Kanters Date: Fri, 22 Nov 2024 22:19:48 +0100 Subject: [PATCH 25/26] fix typing --- lib/extension/externalConverters.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/extension/externalConverters.ts b/lib/extension/externalConverters.ts index 79eefe0f69..5c54bbbae2 100644 --- a/lib/extension/externalConverters.ts +++ b/lib/extension/externalConverters.ts @@ -1,9 +1,11 @@ +import type * as zhc from 'zigbee-herdsman-converters'; + import {addDefinition, removeExternalDefinitions} from 'zigbee-herdsman-converters'; import logger from '../util/logger'; import ExternalJSExtension from './externalJS'; -type ModuleExports = ExternalDefinition | ExternalDefinition[]; +type ModuleExports = zhc.Definition | zhc.Definition[]; export default class ExternalConverters extends ExternalJSExtension { constructor( @@ -60,7 +62,7 @@ export default class ExternalConverters extends ExternalJSExtension Date: Fri, 22 Nov 2024 22:56:12 +0100 Subject: [PATCH 26/26] Cleanup `external_converters` setting remnants. --- lib/types/types.d.ts | 1 - lib/util/settings.schema.json | 10 ---------- lib/util/settings.ts | 1 - test/extensions/bridge.test.ts | 5 ++--- test/mocks/data.ts | 1 - test/settings.test.ts | 5 ++--- 6 files changed, 4 insertions(+), 19 deletions(-) diff --git a/lib/types/types.d.ts b/lib/types/types.d.ts index 91169bdb39..b030dc57b7 100644 --- a/lib/types/types.d.ts +++ b/lib/types/types.d.ts @@ -115,7 +115,6 @@ declare global { active: {timeout: number}; passive: {timeout: number}; }; - external_converters: string[]; mqtt: { base_topic: string; include_device_information: boolean; diff --git a/lib/util/settings.schema.json b/lib/util/settings.schema.json index aee261a5fe..6618dfa4ee 100644 --- a/lib/util/settings.schema.json +++ b/lib/util/settings.schema.json @@ -86,16 +86,6 @@ "requiresRestart": true, "description": "Checks whether devices are online/offline" }, - "external_converters": { - "type": "array", - "title": "External converters", - "description": "You can define external converters to e.g. add support for a DiY device", - "requiresRestart": true, - "items": { - "type": "string" - }, - "examples": ["DIYRuZ_FreePad.js"] - }, "mqtt": { "type": "object", "title": "MQTT", diff --git a/lib/util/settings.ts b/lib/util/settings.ts index 9f98539f98..4430bdc30a 100644 --- a/lib/util/settings.ts +++ b/lib/util/settings.ts @@ -25,7 +25,6 @@ const ajvRestartRequiredGroupOptions = new Ajv({allErrors: true}) .addKeyword({keyword: 'requiresRestart', validate: (s: unknown) => !s}) .compile(schemaJson.definitions.group); const defaults: RecursivePartial = { - external_converters: [], mqtt: { base_topic: 'zigbee2mqtt', include_device_information: false, diff --git a/test/extensions/bridge.test.ts b/test/extensions/bridge.test.ts index 8be3c3f9ad..426a29d836 100644 --- a/test/extensions/bridge.test.ts +++ b/test/extensions/bridge.test.ts @@ -181,7 +181,6 @@ describe('Extension: Bridge', () => { retain: false, }, }, - external_converters: [], groups: { 1: {friendly_name: 'group_1', retain: false}, 11: {friendly_name: 'group_with_tradfri', retain: false}, @@ -3781,11 +3780,11 @@ describe('Extension: Bridge', () => { it('Change options not valid against schema', async () => { mockMQTT.publishAsync.mockClear(); - mockMQTTEvents.message('zigbee2mqtt/bridge/request/options', stringify({options: {external_converters: 'true'}})); + mockMQTTEvents.message('zigbee2mqtt/bridge/request/options', stringify({options: {advanced: {log_level: 123}}})); await flushPromises(); expect(mockMQTT.publishAsync).toHaveBeenCalledWith( 'zigbee2mqtt/bridge/response/options', - stringify({data: {}, error: 'external_converters must be array', status: 'error'}), + stringify({data: {}, error: 'advanced/log_level must be string', status: 'error'}), {retain: false, qos: 0}, ); }); diff --git a/test/mocks/data.ts b/test/mocks/data.ts index 92d2b8040c..c2e1b17404 100644 --- a/test/mocks/data.ts +++ b/test/mocks/data.ts @@ -234,7 +234,6 @@ export function writeDefaultConfiguration(config: unknown = undefined): void { friendly_name: 'ha_discovery_group', }, }, - external_converters: [], }; yaml.writeIfChanged(path.join(mockDir, 'configuration.yaml'), config); diff --git a/test/settings.test.ts b/test/settings.test.ts index 62a5f79d6a..307a41ae26 100644 --- a/test/settings.test.ts +++ b/test/settings.test.ts @@ -15,7 +15,6 @@ const devicesFile2 = mockedData.joinPath('devices2.yaml'); const groupsFile = mockedData.joinPath('groups.yaml'); const secretFile = mockedData.joinPath('secret.yaml'); const minimalConfig = { - external_converters: [], homeassistant: true, mqtt: {base_topic: 'zigbee2mqtt', server: 'localhost'}, }; @@ -64,13 +63,13 @@ describe('Settings', () => { }); it('Should return settings', () => { - write(configurationFile, {external_converters: ['abcd.js']}); + write(configurationFile, {serial: {disable_led: true}}); const s = settings.get(); // @ts-expect-error workaround const expected = objectAssignDeep.noMutate({}, settings.testing.defaults); expected.devices = {}; expected.groups = {}; - expected.external_converters = ['abcd.js']; + expected.serial = {disable_led: true}; expect(s).toStrictEqual(expected); });