From c3b2e7738c0081dd4dc8d98b2e60482745924c60 Mon Sep 17 00:00:00 2001 From: Max Hauser Date: Tue, 10 Sep 2024 14:46:15 +0200 Subject: [PATCH] Cleanup context data notifications (#2904) * prevent having too many args on public methods in the future by introducing options objects * fix jsdoc * added notification and made structure a bit more clear * fix types --- README.md | 6 + packages/adapter/src/lib/_Types.ts | 10 ++ packages/adapter/src/lib/adapter/adapter.ts | 24 ++-- packages/cli/src/lib/setup.ts | 12 +- .../src/lib/common/notificationHandler.ts | 38 +++--- packages/controller/src/main.ts | 120 +++++++++++------- packages/types-dev/index.d.ts | 10 +- packages/types-dev/objects.d.ts | 2 +- 8 files changed, 133 insertions(+), 89 deletions(-) diff --git a/README.md b/README.md index c85162cb1..58eb39d7d 100644 --- a/README.md +++ b/README.md @@ -512,6 +512,12 @@ This method takes the following parameters: * scope: scope to be addressed * category: category to be addressed, if a null message will be checked by regex of given scope * message: message to be stored/checked +* options: Available with js-controller version 6.1. Additional options for the notification, currently you can provide additional `contextData` which is also stored with the notification information. Notification processing adapters can use this data + +Note, that the structure of the `contextData` which can be stored via the options object is not defined by the controller. Adapters which handle messages can use individual data attributes. +Currently, it is planned to support individual notification customization in the `admin` adapter. More information will be available in the `admin` adapter as soon as this feature is ready. + +As a best practice the top-level of `contextData` should not be populated with individual data belonging to instances. Use a `key` specific to the adapter or if a feature is supported by all adapters of a type, the type (e.g. `messaging`) is also fine. When a regex is defined then `console.error` output from the adapter is always checked by the regex and notifications are registered automatically when the regex matches! diff --git a/packages/adapter/src/lib/_Types.ts b/packages/adapter/src/lib/_Types.ts index 53517bb5a..49d9e1708 100644 --- a/packages/adapter/src/lib/_Types.ts +++ b/packages/adapter/src/lib/_Types.ts @@ -598,3 +598,13 @@ export interface InternalInstallNodeModuleOptions extends InstallNodeModuleOptio /** Name of the npm module or an installable url ẁorking with `npm install` */ moduleNameOrUrl: string; } + +/** + * Options for the generated notification + */ +export interface NotificationOptions { + /** + * Additional context for the notification which can be used by notification processing adapters + */ + contextData: ioBroker.NotificationContextData; +} diff --git a/packages/adapter/src/lib/adapter/adapter.ts b/packages/adapter/src/lib/adapter/adapter.ts index b5ae1468a..4bc9231eb 100644 --- a/packages/adapter/src/lib/adapter/adapter.ts +++ b/packages/adapter/src/lib/adapter/adapter.ts @@ -132,7 +132,8 @@ import type { InstallNodeModuleOptions, InternalInstallNodeModuleOptions, StopParameters, - InternalStopParameters + InternalStopParameters, + NotificationOptions } from '@/lib/_Types.js'; import { UserInterfaceMessagingController } from '@/lib/adapter/userInterfaceMessagingController.js'; import { SYSTEM_ADAPTER_PREFIX } from '@iobroker/js-controller-common-db/constants'; @@ -7589,10 +7590,8 @@ export class AdapterClass extends EventEmitter { registerNotification( scope: Scope, category: ioBroker.NotificationScopes[Scope] | null, - /** Static message (e.g. for messengers) */ message: string, - /** Dynamic message to be shown in admin. This object will be sent to instance to build the schema for dynamic layout (without `offlineMessage`) */ - contextData?: ioBroker.NotificationAction + options?: NotificationOptions ): Promise; /** @@ -7600,15 +7599,10 @@ export class AdapterClass extends EventEmitter { * * @param scope - scope to be addressed * @param category - to be addressed, if a null message will be checked by regex of given scope - * @param message - message to be stored/checked for messangers - * @param contextData - Information for the notification action in Admin + * @param message - message to be stored/checked + * @param options - Additional options for the notification, currently `contextData` is supported */ - async registerNotification( - scope: unknown, - category: unknown, - message: unknown, - contextData?: unknown - ): Promise { + async registerNotification(scope: unknown, category: unknown, message: unknown, options?: unknown): Promise { if (!this.#states) { // if states is no longer existing, we do not need to set this._logger.info( @@ -7623,6 +7617,10 @@ export class AdapterClass extends EventEmitter { } Validator.assertString(message, 'message'); + if (options !== undefined) { + Validator.assertObject(options, 'options'); + } + const obj = { command: 'addNotification', message: { @@ -7630,7 +7628,7 @@ export class AdapterClass extends EventEmitter { category, message, instance: this.namespace, - contextData + contextData: options?.contextData }, from: `system.adapter.${this.namespace}` }; diff --git a/packages/cli/src/lib/setup.ts b/packages/cli/src/lib/setup.ts index 84b4394f4..c06c78d97 100644 --- a/packages/cli/src/lib/setup.ts +++ b/packages/cli/src/lib/setup.ts @@ -755,12 +755,12 @@ async function processCommand( ); await notificationHandler.addConfig(ioPackage.notifications); - await notificationHandler.addMessage( - 'system', - 'fileToJsonl', - `Migrated: ${migrated}`, - `system.host.${hostname}` - ); + await notificationHandler.addMessage({ + scope: 'system', + category: 'fileToJsonl', + message: `Migrated: ${migrated}`, + instance: `system.host.${hostname}` + }); notificationHandler.storeNotifications(); } catch (e) { diff --git a/packages/common/src/lib/common/notificationHandler.ts b/packages/common/src/lib/common/notificationHandler.ts index 0e8e4bfed..a4ac6bf5c 100644 --- a/packages/common/src/lib/common/notificationHandler.ts +++ b/packages/common/src/lib/common/notificationHandler.ts @@ -46,7 +46,7 @@ export interface CategoryConfigEntry { interface NotificationMessageObject { message: string; ts: number; - contextData?: ioBroker.NotificationAction; + contextData?: ioBroker.NotificationContextData; } interface NotificationsObject { @@ -100,6 +100,19 @@ interface ScopeStateValue { }; } +interface AddMessageOptions { + /** Scope of the message */ + scope: string; + /** Category of the message, if non we check against regex of scope */ + category?: string | null; + /** Message to add */ + message: string; + /** Instance e.g., hm-rpc.1 or hostname, if hostname it needs to be prefixed like system.host.rpi */ + instance: string; + /** Additional context for the notification which can be used by notification processing adapters */ + contextData?: ioBroker.NotificationContextData; +} + export class NotificationHandler { private states: StatesInRedisClient; private objects: ObjectsInRedisClient; @@ -129,14 +142,14 @@ export class NotificationHandler { // create the initial notifications object let obj; try { - obj = await this.objects.getObjectAsync(`system.host.${this.host}.notifications`); + obj = await this.objects.getObject(`system.host.${this.host}.notifications`); } catch { // ignore } if (!obj) { try { - await this.objects.setObjectAsync(`system.host.${this.host}.notifications`, { + await this.objects.setObject(`system.host.${this.host}.notifications`, { type: 'folder', common: { name: { @@ -203,7 +216,7 @@ export class NotificationHandler { /** * Add a new category to the given scope with a provided optional list of regex * - * @param notifications Array with notifications + * @param notifications - Array with notifications */ async addConfig(notifications: NotificationsConfigEntry[]): Promise { // if valid attributes, store it @@ -284,19 +297,12 @@ export class NotificationHandler { /** * Add a message to the scope and category * - * @param scope - scope of the message - * @param category - category of the message, if non we check against regex of scope - * @param message - message to add - * @param instance - instance e.g., hm-rpc.1 or hostname, if hostname it needs to be prefixed like system.host.rpi - * @param contextData - data for the notification action + * @param options The scope, category, message, instance and contextData information */ - async addMessage( - scope: string, - category: string | null | undefined, - message: string, - instance: string, - contextData?: ioBroker.NotificationAction - ): Promise { + async addMessage(options: AddMessageOptions): Promise { + const { message, scope, category, contextData } = options; + let { instance } = options; + if (typeof instance !== 'string') { this.log.error( `${this.logPrefix} [addMessage] Instance has to be of type "string", got "${typeof instance}"` diff --git a/packages/controller/src/main.ts b/packages/controller/src/main.ts index 1c7cabf89..c3fc54e1f 100644 --- a/packages/controller/src/main.ts +++ b/packages/controller/src/main.ts @@ -987,13 +987,14 @@ async function checkSystemLocaleSupported(): Promise { const isSupported = await objects.isSystemLocaleSupported(); if (!isSupported) { - await notificationHandler.addMessage( - 'system', - 'databaseErrors', - 'Your redis server is using an unsupported locale. This can lead to unexpected behavior of your ioBroker installation as well as data loss. ' + + await notificationHandler.addMessage({ + category: 'system', + scope: 'databaseErrors', + message: + 'Your redis server is using an unsupported locale. This can lead to unexpected behavior of your ioBroker installation as well as data loss. ' + 'Please configure your Redis Server according to https://forum.iobroker.net/topic/52976/wichtiger-hinweis-f%C3%BCr-redis-installationen?_=1678099836122', - `system.host.${hostname}` - ); + instance: `system.host.${hostname}` + }); } } @@ -1126,12 +1127,12 @@ async function reportStatus(): Promise { const isDiskWarningActive = percentageFree < diskWarningLevel; if (isDiskWarningActive) { - await notificationHandler.addMessage( - 'system', - 'diskSpaceIssues', - `Your system has only ${percentageFree.toFixed(2)} % of disk space left.`, - `system.host.${hostname}` - ); + await notificationHandler.addMessage({ + scope: 'system', + category: 'diskSpaceIssues', + message: `Your system has only ${percentageFree.toFixed(2)} % of disk space left.`, + instance: `system.host.${hostname}` + }); } states.setState(`${id}.diskSize`, { @@ -2924,13 +2925,14 @@ async function processMessage(msg: ioBroker.SendableMessage): Promise { return; } - await notificationHandler.addMessage('system', 'packageUpdates', packages.join('\n'), `system.host.${hostname}`); + await notificationHandler.addMessage({ + scope: 'system', + category: 'packageUpdates', + message: packages.join('\n'), + instance: `system.host.${hostname}` + }); } /** @@ -5832,7 +5844,12 @@ async function checkRebootRequired(): Promise { } } - await notificationHandler.addMessage('system', 'systemRebootRequired', message, `system.host.${hostname}`); + await notificationHandler.addMessage({ + scope: 'system', + category: 'systemRebootRequired', + message, + instance: `system.host.${hostname}` + }); } /** @@ -5848,21 +5865,25 @@ async function autoUpgradeAdapters(): Promise { const { upgradedAdapters, failedAdapters } = await autoUpgradeManager.upgradeAdapters(); if (upgradedAdapters.length) { - await notificationHandler.addMessage( - 'system', - 'automaticAdapterUpgradeSuccessful', - upgradedAdapters.map(entry => `${entry.name}: ${entry.oldVersion} -> ${entry.newVersion}`).join('\n'), - `system.host.${hostname}` - ); + await notificationHandler.addMessage({ + scope: 'system', + category: 'automaticAdapterUpgradeSuccessful', + message: upgradedAdapters + .map(entry => `${entry.name}: ${entry.oldVersion} -> ${entry.newVersion}`) + .join('\n'), + instance: `system.host.${hostname}` + }); } if (failedAdapters.length) { - await notificationHandler.addMessage( - 'system', - 'automaticAdapterUpgradeFailed', - failedAdapters.map(entry => `${entry.name}: ${entry.oldVersion} -> ${entry.newVersion}`).join('\n'), - `system.host.${hostname}` - ); + await notificationHandler.addMessage({ + scope: 'system', + category: 'automaticAdapterUpgradeFailed', + message: failedAdapters + .map(entry => `${entry.name}: ${entry.oldVersion} -> ${entry.newVersion}`) + .join('\n'), + instance: `system.host.${hostname}` + }); } } catch (e) { logger.error(`${hostLogPrefix} An error occurred while processing automatic adapter upgrades: ${e.message}`); @@ -5886,7 +5907,12 @@ async function disableBlocklistedInstances(): Promise { const message = `Instance "${disabledInstance._id}" has been stopped and disabled because the version "${disabledInstance.common.version}" has been blocked by the developer`; logger.error(`${hostLogPrefix} ${message}`); - await notificationHandler.addMessage('system', 'blockedVersions', message, SYSTEM_HOST_PREFIX + hostname); + await notificationHandler.addMessage({ + scope: 'system', + category: 'blockedVersions', + message, + instance: SYSTEM_HOST_PREFIX + hostname + }); } } diff --git a/packages/types-dev/index.d.ts b/packages/types-dev/index.d.ts index 7d34f615b..146a673ab 100644 --- a/packages/types-dev/index.d.ts +++ b/packages/types-dev/index.d.ts @@ -324,12 +324,10 @@ declare global { [other: string]: string; } - /** Structure for notification actions */ - export interface NotificationAction { - /** This message will be shown if instance is offline */ - offlineMessage?: ioBroker.StringOrTranslated; - /** any other data required for instance to show the dynamic GUI in the notification */ - [other: string]: any; + /** Additional context for the notification which can be used by notification processing adapters */ + interface NotificationContextData { + /** Use a `key` specific to the adapter or if a feature is supported by all adapters of a type, the type (e.g. `messaging`) is also fine. */ + [adapterNameOrAdapterType: string]: unknown; } interface AdapterConfig { diff --git a/packages/types-dev/objects.d.ts b/packages/types-dev/objects.d.ts index db4fcc6c9..8d90a123c 100644 --- a/packages/types-dev/objects.d.ts +++ b/packages/types-dev/objects.d.ts @@ -1089,7 +1089,7 @@ declare global { }; interface Notification { - /** e.g., `system`. Each adapter can define its own "scopes" for own notifications with its own categories which then will be available in the system. Adapters should only register one scope which matches the name of the adapter. */ + /** E.g., `system`. Each adapter can define its own "scopes" for own notifications with its own categories which then will be available in the system. Adapters should only register one scope which matches the name of the adapter. */ scope: string; /** The human-readable name of this scope */ name: Translated;