Skip to content

Commit

Permalink
Cleanup context data notifications (#2904)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
foxriver76 authored Sep 10, 2024
1 parent 5fef76d commit c3b2e77
Show file tree
Hide file tree
Showing 8 changed files with 133 additions and 89 deletions.
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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!

Expand Down
10 changes: 10 additions & 0 deletions packages/adapter/src/lib/_Types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
24 changes: 11 additions & 13 deletions packages/adapter/src/lib/adapter/adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -7589,26 +7590,19 @@ export class AdapterClass extends EventEmitter {
registerNotification<Scope extends keyof ioBroker.NotificationScopes>(
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<void>;

/**
* Send notification with given scope and category to host of this adapter
*
* @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<void> {
async registerNotification(scope: unknown, category: unknown, message: unknown, options?: unknown): Promise<void> {
if (!this.#states) {
// if states is no longer existing, we do not need to set
this._logger.info(
Expand All @@ -7623,14 +7617,18 @@ export class AdapterClass extends EventEmitter {
}
Validator.assertString(message, 'message');

if (options !== undefined) {
Validator.assertObject<NotificationOptions>(options, 'options');
}

const obj = {
command: 'addNotification',
message: {
scope,
category,
message,
instance: this.namespace,
contextData
contextData: options?.contextData
},
from: `system.adapter.${this.namespace}`
};
Expand Down
12 changes: 6 additions & 6 deletions packages/cli/src/lib/setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
38 changes: 22 additions & 16 deletions packages/common/src/lib/common/notificationHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export interface CategoryConfigEntry {
interface NotificationMessageObject {
message: string;
ts: number;
contextData?: ioBroker.NotificationAction;
contextData?: ioBroker.NotificationContextData;
}

interface NotificationsObject {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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: {
Expand Down Expand Up @@ -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<void> {
// if valid attributes, store it
Expand Down Expand Up @@ -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<void> {
async addMessage(options: AddMessageOptions): Promise<void> {
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}"`
Expand Down
120 changes: 73 additions & 47 deletions packages/controller/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -987,13 +987,14 @@ async function checkSystemLocaleSupported(): Promise<void> {
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}`
});
}
}

Expand Down Expand Up @@ -1126,12 +1127,12 @@ async function reportStatus(): Promise<void> {
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`, {
Expand Down Expand Up @@ -2924,13 +2925,14 @@ async function processMessage(msg: ioBroker.SendableMessage): Promise<null | voi
}

case 'addNotification':
await notificationHandler.addMessage(
msg.message.scope,
msg.message.category,
msg.message.message,
msg.message.instance,
msg.message.contextData
);
await notificationHandler.addMessage({
scope: msg.message.scope,
category: msg.message.category,
message: msg.message.message,
instance: msg.message.instance,
contextData: msg.message.contextData
});

if (msg.callback && msg.from) {
sendTo(msg.from, msg.command, { result: 'ok' }, msg.callback);
}
Expand Down Expand Up @@ -3804,7 +3806,12 @@ async function startInstance(id: ioBroker.ObjectIDs.Instance, wakeUp = false): P
const message = `Do not start instance "${id}", because the version "${instance.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
});
return;
}

Expand Down Expand Up @@ -3945,12 +3952,12 @@ async function startInstance(id: ioBroker.ObjectIDs.Instance, wakeUp = false): P

// add it to notifications for popup
try {
await notificationHandler.addMessage(
'system',
'memIssues',
`Your system has only ${availableMemMB} MB RAM left available and an additional adapter process is started. Please check your system, settings and active instances to prevent swapping and Out-Of-Memory situations!`,
`system.host.${hostname}`
);
await notificationHandler.addMessage({
scope: 'system',
category: 'memIssues',
message: `Your system has only ${availableMemMB} MB RAM left available and an additional adapter process is started. Please check your system, settings and active instances to prevent swapping and Out-Of-Memory situations!`,
instance: `system.host.${hostname}`
});
} catch (e) {
logger.warn(`${hostLogPrefix} Could not add OOM notification: ${e.message}`);
}
Expand Down Expand Up @@ -4207,12 +4214,12 @@ async function startInstance(id: ioBroker.ObjectIDs.Instance, wakeUp = false): P
logger.warn(
`${hostLogPrefix} Do not restart adapter ${id} because restart loop detected`
);
await notificationHandler.addMessage(
'system',
'restartLoop',
'Restart loop detected',
id
);
await notificationHandler.addMessage({
scope: 'system',
category: 'restartLoop',
message: 'Restart loop detected',
instance: id
});
proc.crashCount = 0;
if (proc.crashResetTimer) {
logger.debug(
Expand Down Expand Up @@ -5748,7 +5755,12 @@ async function listUpdatableOsPackages(): Promise<void> {
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}`
});
}

/**
Expand Down Expand Up @@ -5832,7 +5844,12 @@ async function checkRebootRequired(): Promise<void> {
}
}

await notificationHandler.addMessage('system', 'systemRebootRequired', message, `system.host.${hostname}`);
await notificationHandler.addMessage({
scope: 'system',
category: 'systemRebootRequired',
message,
instance: `system.host.${hostname}`
});
}

/**
Expand All @@ -5848,21 +5865,25 @@ async function autoUpgradeAdapters(): Promise<void> {
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}`);
Expand All @@ -5886,7 +5907,12 @@ async function disableBlocklistedInstances(): Promise<void> {
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
});
}
}

Expand Down
10 changes: 4 additions & 6 deletions packages/types-dev/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading

0 comments on commit c3b2e77

Please sign in to comment.