diff --git a/src/device/knownDeviceRegistry.ts b/src/device/knownDeviceRegistry.ts new file mode 100644 index 00000000..748818f5 --- /dev/null +++ b/src/device/knownDeviceRegistry.ts @@ -0,0 +1,63 @@ +import Settings from '../settings/settings.js'; +import KnownDevice from '../settings/knownDevice.js'; +import DeviceNameGenerator from './deviceNameGenerator.js'; +import Logger from '../logging/Logger.js'; +import { DeviceId } from './deviceId.js'; + +/** + * Looks up and registers the persisted `KnownDevice` identity for a newly detected raw device + * (serial port, BLE peripheral, buttplug.io device, ...). + * + * Centralizes identity lookup/creation logic that used to be duplicated across several device + * providers/factories. Deliberately has no opinion on *when* a newly created identity should be + * persisted - `resolve()` never has side effects, so callers stay in control of only calling + * `persist()` once they've actually finished building the Device successfully. + */ +export default class KnownDeviceRegistry +{ + private readonly settings: Settings; + + private readonly nameGenerator: DeviceNameGenerator; + + private readonly logger: Logger; + + public constructor(settings: Settings, nameGenerator: DeviceNameGenerator, logger: Logger) { + this.settings = settings; + this.nameGenerator = nameGenerator; + this.logger = logger.child({ name: KnownDeviceRegistry.name }); + } + + /** + * Looks up the already-known identity for `deviceId`, or builds a new (not yet persisted) + * one if none exists. + */ + public resolve(deviceId: DeviceId, type: string, provider: string, name?: string): KnownDevice { + const knownDevice = this.settings.getKnownDeviceById(deviceId); + + if (undefined !== knownDevice) { + // Already known (previously detected serial number) + this.logger.debug(`Device is already known: ${knownDevice.id}`); + return knownDevice; + } + + return new KnownDevice(deviceId, name ?? this.nameGenerator.generateName(), type, provider); + } + + /** + * Persists a resolved identity. Safe to call unconditionally after successfully building a + * Device, even for an already-known identity - a no-op in that case, since KnownDevice is + * immutable and `resolve()` returns the exact same instance for an already-known device. + * + * This matters beyond just avoiding pointless work: Settings is wrapped with `on-change` to + * auto-save to disk, so an unconditional `settings.addKnownDevice()` call here would trigger a + * settings.json write and a settings-changed WebSocket broadcast on *every* device connect, + * even for a device that has been known and unchanged for months. + */ + public persist(knownDevice: KnownDevice): void { + if (this.settings.getKnownDeviceById(knownDevice.id) === knownDevice) { + return; + } + + this.settings.addKnownDevice(knownDevice); + } +} diff --git a/src/device/protocol/airotic/airoticDeviceProvider.ts b/src/device/protocol/airotic/airoticDeviceProvider.ts index 12f8730f..dca1649a 100644 --- a/src/device/protocol/airotic/airoticDeviceProvider.ts +++ b/src/device/protocol/airotic/airoticDeviceProvider.ts @@ -10,27 +10,32 @@ import AiroticProtocol from './airtonicProtocol.js'; import MessageResponseHandler from '../messageResponseHandler.js'; import StrDeviceAttribute from '../../attribute/strDeviceAttribute.js'; import { DeviceAttributeModifier } from '../../attribute/deviceAttribute.js'; -import Settings from '../../../settings/settings.js'; -import KnownDevice from '../../../settings/knownDevice.js'; -import { DeviceId } from '../../deviceId.js'; +import KnownDeviceRegistry from '../../knownDeviceRegistry.js'; import BoolDeviceAttribute from '../../attribute/boolDeviceAttribute.js'; import FloatDeviceAttribute from '../../attribute/floatDeviceAttribute.js'; import BleDeviceProvider from '../../provider/bleDeviceProvider.js'; import { hsvByteToRgb } from '../../../util/color.js'; +import { NoDeviceProviderConfig } from '../../provider/deviceProviderConfig.js'; -export default class AiroticDeviceProvider extends BleDeviceProvider +export default class AiroticDeviceProvider extends BleDeviceProvider { public static readonly providerName = 'airotic'; private static readonly UART_RX_CHAR_UUID = '6e400002b5a3f393e0a9e50e24dcca9e'; private static readonly UART_TX_CHAR_UUID = '6e400003b5a3f393e0a9e50e24dcca9e'; - private readonly settings: Settings; + private readonly knownDeviceRegistry: KnownDeviceRegistry; - public constructor(deviceManager: DeviceManager, settings: Settings, eventEmitter: EventEmitter, logger: Logger) { - super(deviceManager, eventEmitter, logger.child({ name: AiroticDeviceProvider.name })); + public constructor( + config: NoDeviceProviderConfig, + deviceManager: DeviceManager, + knownDeviceRegistry: KnownDeviceRegistry, + eventEmitter: EventEmitter, + logger: Logger + ) { + super(config, deviceManager, eventEmitter, logger.child({ name: AiroticDeviceProvider.name })); - this.settings = settings; + this.knownDeviceRegistry = knownDeviceRegistry; } public override async init(): Promise { @@ -56,8 +61,10 @@ export default class AiroticDeviceProvider extends BleDeviceProvider; diff --git a/src/device/protocol/buttplugIo/buttplugIoWebsocketDeviceProvider.ts b/src/device/protocol/buttplugIo/buttplugIoWebsocketDeviceProvider.ts index 6438929d..f4942fec 100644 --- a/src/device/protocol/buttplugIo/buttplugIoWebsocketDeviceProvider.ts +++ b/src/device/protocol/buttplugIo/buttplugIoWebsocketDeviceProvider.ts @@ -9,40 +9,32 @@ import SlvCtrlPlusButtplugWebsocketClientConnector from './slvCtrlPlusButtplugWe import DeviceManager from '../../deviceManager.js'; import { logError } from '../../../util/error.js'; import { hasProperty } from '../../../util/objects.js'; +import { ButtplugIoWebsocketConfig } from './buttplugIoWebsocketConfig.js'; -export default class ButtplugIoWebsocketDeviceProvider extends DeviceProvider { +export default class ButtplugIoWebsocketDeviceProvider extends DeviceProvider { public static readonly providerName = 'buttplugIoWebsocket'; private connectedDevices: Map = new Map(); - private buttplugConnector: ButtplugNodeWebsocketClientConnector; - private buttplugClient: ButtplugClient; + private readonly buttplugConnector: ButtplugNodeWebsocketClientConnector; + private readonly buttplugClient: ButtplugClient; private readonly buttplugIoDeviceFactory: ButtplugIoDeviceFactory; - private readonly websocketAddress: string; - private readonly autoScan: boolean; - private readonly useDeviceNameAsId: boolean; - private connectionIntervalRef?: NodeJS.Timeout; private autoScanningIntervalRef?: NodeJS.Timeout; public constructor( + config: ButtplugIoWebsocketConfig, deviceManager: DeviceManager, eventEmitter: EventEmitter, deviceFactory: ButtplugIoDeviceFactory, - websocketAddress: string, - autoScan: boolean, - useDeviceNameAsId: boolean, logger: Logger ) { - super(deviceManager, eventEmitter, logger.child({ name: ButtplugIoWebsocketDeviceProvider.name })); + super(config, deviceManager, eventEmitter, logger.child({ name: ButtplugIoWebsocketDeviceProvider.name })); this.buttplugIoDeviceFactory = deviceFactory; - this.websocketAddress = websocketAddress; - this.autoScan = autoScan; - this.useDeviceNameAsId = useDeviceNameAsId; - const url = `ws://${this.websocketAddress}/buttplug`; + const url = `ws://${this.config.address}/buttplug`; this.buttplugConnector = new SlvCtrlPlusButtplugWebsocketClientConnector(url); this.buttplugClient = new ButtplugClient('SlvCtrlPlus'); @@ -66,7 +58,7 @@ export default class ButtplugIoWebsocketDeviceProvider extends DeviceProvider { return; } - const url = `ws://${this.websocketAddress}/buttplug`; + const url = `ws://${this.config.address}/buttplug`; try { await this.buttplugClient.connect(this.buttplugConnector); @@ -75,7 +67,7 @@ export default class ButtplugIoWebsocketDeviceProvider extends DeviceProvider { clearInterval(this.connectionIntervalRef); this.connectionIntervalRef = undefined; - if (this.autoScan) { + if (this.config.autoScan) { this.autoScanningIntervalRef ??= setImmediateInterval(() => { this.discoverButtplugIoDevices() }, 60000); } } catch (e: unknown) { @@ -108,7 +100,7 @@ export default class ButtplugIoWebsocketDeviceProvider extends DeviceProvider { .catch((e: unknown) => this.logger.error(`Could not start scanning for buttplug.io devices`, e)); setTimeout(() => { - if (undefined === this.buttplugClient || !this.buttplugClient.isScanning) { + if (!this.buttplugClient.isScanning) { return; } @@ -122,7 +114,7 @@ export default class ButtplugIoWebsocketDeviceProvider extends DeviceProvider { this.logger.info(`Device detected: ${buttplugDevice.name}`, buttplugDevice); try { - const device = this.buttplugIoDeviceFactory.create(buttplugDevice, ButtplugIoWebsocketDeviceProvider.providerName, this.useDeviceNameAsId); + const device = this.buttplugIoDeviceFactory.create(buttplugDevice, ButtplugIoWebsocketDeviceProvider.providerName, this.config.useDeviceNameAsId); this.connectedDevices.set(buttplugDevice.index, device); diff --git a/src/device/protocol/buttplugIo/buttplugIoWebsocketDeviceProviderFactory.ts b/src/device/protocol/buttplugIo/buttplugIoWebsocketDeviceProviderFactory.ts deleted file mode 100644 index 8f7064d6..00000000 --- a/src/device/protocol/buttplugIo/buttplugIoWebsocketDeviceProviderFactory.ts +++ /dev/null @@ -1,48 +0,0 @@ -import EventEmitter from 'events'; -import DeviceProviderFactory from '../../provider/deviceProviderFactory.js'; -import Logger from '../../../logging/Logger.js'; -import ButtplugIoDeviceFactory from './buttplugIoDeviceFactory.js'; -import ButtplugIoWebsocketDeviceProvider from './buttplugIoWebsocketDeviceProvider.js'; -import DeviceManager from '../../deviceManager.js'; - -type ButtplugIoWebsocketConfig = { - address: string, - autoScan: boolean, - useDeviceNameAsId: boolean -} - -export default class ButtplugIoWebsocketDeviceProviderFactory implements DeviceProviderFactory -{ - private readonly deviceManager: DeviceManager; - - private readonly eventEmitter: EventEmitter; - - private readonly deviceFactory: ButtplugIoDeviceFactory; - - private readonly logger: Logger; - - public constructor( - deviceManager: DeviceManager, - eventEmitter: EventEmitter, - deviceFactory: ButtplugIoDeviceFactory, - logger: Logger - ) { - this.deviceManager = deviceManager; - this.eventEmitter = eventEmitter; - this.deviceFactory = deviceFactory; - this.logger = logger; - } - - public create(config: ButtplugIoWebsocketConfig): ButtplugIoWebsocketDeviceProvider - { - return new ButtplugIoWebsocketDeviceProvider( - this.deviceManager, - this.eventEmitter, - this.deviceFactory, - config.address, - config.autoScan, - config.useDeviceNameAsId, - this.logger - ); - } -} diff --git a/src/device/protocol/estim2b/estim2bSerialDeviceProvider.ts b/src/device/protocol/estim2b/estim2bSerialDeviceProvider.ts index e16f64c6..08592ca7 100644 --- a/src/device/protocol/estim2b/estim2bSerialDeviceProvider.ts +++ b/src/device/protocol/estim2b/estim2bSerialDeviceProvider.ts @@ -13,8 +13,9 @@ import SerialDeviceTransportFactory from '../../transport/serialDeviceTransportF import { getErrorFromDecodeResult } from '../deviceProtocol.js'; import DeviceManager from '../../deviceManager.js'; import { SerialDeviceInfo } from '../../transport/serialPortObserver.js'; +import { NoDeviceProviderConfig } from '../../provider/deviceProviderConfig.js'; -export default class EStim2bSerialDeviceProvider extends SerialDeviceProvider +export default class EStim2bSerialDeviceProvider extends SerialDeviceProvider { public static readonly providerName = 'estim2bSerial'; @@ -23,6 +24,7 @@ export default class EStim2bSerialDeviceProvider extends SerialDeviceProvider { const deviceInfo = await this.getDeviceInfo(transport); const protocol = deviceInfo.protocol; - const knownDevice = this.createKnownDevice(deviceId, deviceInfo.deviceType, provider); + const knownDevice = this.knownDeviceRegistry.resolve(deviceId, deviceInfo.deviceType, provider); const deviceAttributes = await this.getAttributes(transport, protocol); const device = new GenericSlvCtrlPlusDevice( @@ -60,7 +54,7 @@ export default class SlvCtrlPlusDeviceFactory this.logger, ); - this.settings.addKnownDevice(knownDevice); + this.knownDeviceRegistry.persist(knownDevice); return device; } @@ -125,22 +119,4 @@ export default class SlvCtrlPlusDeviceFactory return new SlvCtrlProtocolV1(); } - - private createKnownDevice(deviceId: DeviceId, deviceType: string, provider: string): KnownDevice { - const knownDevice = this.settings.getKnownDeviceById(deviceId) - - if (undefined !== knownDevice) { - // Return already existing device if already known (previously detected serial number) - this.logger.debug(`Device is already known: ${knownDevice.id}`); - return knownDevice; - } - - // Create a new device and return if not yet known (new serial number) - return new KnownDevice( - deviceId, - this.nameGenerator.generateName(), - deviceType, - provider - ); - } } diff --git a/src/device/protocol/slvCtrlPlus/slvCtrlPlusSerialDeviceProvider.ts b/src/device/protocol/slvCtrlPlus/slvCtrlPlusSerialDeviceProvider.ts index af1f1445..59a2b0e0 100644 --- a/src/device/protocol/slvCtrlPlus/slvCtrlPlusSerialDeviceProvider.ts +++ b/src/device/protocol/slvCtrlPlus/slvCtrlPlusSerialDeviceProvider.ts @@ -14,8 +14,9 @@ import DeviceBidirectionalTransport from '../../transport/deviceBidirectionalTra import DeviceManager from '../../deviceManager.js'; import GenericSlvCtrlPlusDevice from './genericSlvCtrlPlusDevice.js'; import { SerialDeviceInfo } from '../../transport/serialPortObserver.js'; +import { NoDeviceProviderConfig } from '../../provider/deviceProviderConfig.js'; -export default class SlvCtrlPlusSerialDeviceProvider extends SerialDeviceProvider +export default class SlvCtrlPlusSerialDeviceProvider extends SerialDeviceProvider { public static readonly providerName = 'slvCtrlPlusSerial'; @@ -28,6 +29,7 @@ export default class SlvCtrlPlusSerialDeviceProvider extends SerialDeviceProvide private readonly deviceTransportFactory: SerialDeviceTransportFactory; public constructor( + config: NoDeviceProviderConfig, deviceManager: DeviceManager, serialPortFactory: SerialPortFactory, eventEmitter: EventEmitter, @@ -35,7 +37,7 @@ export default class SlvCtrlPlusSerialDeviceProvider extends SerialDeviceProvide deviceTransportFactory: SerialDeviceTransportFactory, logger: Logger ) { - super(deviceManager, serialPortFactory, eventEmitter, logger.child({ name: SlvCtrlPlusSerialDeviceProvider.name })); + super(config, deviceManager, serialPortFactory, eventEmitter, logger.child({ name: SlvCtrlPlusSerialDeviceProvider.name })); this.slvCtrlPlusDeviceFactory = deviceFactory; this.deviceTransportFactory = deviceTransportFactory; } diff --git a/src/device/protocol/virtual/genericVirtualDeviceFactory.ts b/src/device/protocol/virtual/genericVirtualDeviceFactory.ts index bf95e56f..1a1f4d95 100644 --- a/src/device/protocol/virtual/genericVirtualDeviceFactory.ts +++ b/src/device/protocol/virtual/genericVirtualDeviceFactory.ts @@ -1,4 +1,3 @@ -import { Static, TObject } from '@sinclair/typebox'; import VirtualDeviceLogic from './virtualDeviceLogic.js'; import DateFactory from '../../../factory/dateFactory.js'; import JsonSchemaValidatorFactory from '../../../schemaValidation/JsonSchemaValidatorFactory.js'; @@ -9,19 +8,6 @@ import VirtualDeviceLogicFactory from './virtualDeviceLogicFactory.js'; import Logger from '../../../logging/Logger.js'; import EventEmitterFactory from '../../../factory/eventEmitterFactory.js'; -type ExtractConfig> = T extends VirtualDeviceLogic ? C : never; - -type LogicFactoryAndConfigTuple, TConfigSchema extends TObject> = { - deviceLogicFactory: VirtualDeviceLogicFactory, - deviceConfigSchema: TConfigSchema & ( - Static extends ExtractConfig - ? ExtractConfig extends Static - ? unknown - : never - : never - ), -}; - export default class GenericVirtualDeviceFactory implements VirtualDeviceFactory { private readonly dateFactory: DateFactory; @@ -29,7 +15,7 @@ export default class GenericVirtualDeviceFactory implements VirtualDeviceFactory private readonly jsonSchemaValidatorFactory: JsonSchemaValidatorFactory; - private readonly logicFactories: Map, TObject>> = new Map(); + private readonly logicFactories: Map>> = new Map(); private readonly logger: Logger; @@ -45,17 +31,10 @@ export default class GenericVirtualDeviceFactory implements VirtualDeviceFactory this.logger = logger; } - public addLogicFactory< - TLogic extends VirtualDeviceLogic, - TConfigSchema extends TObject - >( - virtualDeviceLogicFactory: LogicFactoryAndConfigTuple['deviceLogicFactory'], - deviceConfigSchema: LogicFactoryAndConfigTuple['deviceConfigSchema'], + public addLogicFactory>( + virtualDeviceLogicFactory: VirtualDeviceLogicFactory, ): this { - this.logicFactories.set(virtualDeviceLogicFactory.forDeviceType(), { - deviceLogicFactory: virtualDeviceLogicFactory, - deviceConfigSchema, - }); + this.logicFactories.set(virtualDeviceLogicFactory.forDeviceType(), virtualDeviceLogicFactory); return this; } @@ -69,7 +48,7 @@ export default class GenericVirtualDeviceFactory implements VirtualDeviceFactory throw new Error(`Could not find a factory for virtual device logic '${factoryName}'`); } - const jsonSchemaValidator = this.jsonSchemaValidatorFactory.create(factory.deviceConfigSchema); + const jsonSchemaValidator = this.jsonSchemaValidatorFactory.create(factory.configSchema); const isConfigValid = jsonSchemaValidator.validate(knownDevice.config); if (!isConfigValid) { @@ -77,7 +56,7 @@ export default class GenericVirtualDeviceFactory implements VirtualDeviceFactory throw new Error(`Config for device is not valid: ${JSON.stringify(validationErrors, null, 2)}`); } - const deviceLogic = factory.deviceLogicFactory.create(knownDevice.config); + const deviceLogic = factory.create(knownDevice.config); const device = new VirtualDevice( '1.0.0', diff --git a/src/device/protocol/virtual/genericVirtualDeviceLogicFactory.ts b/src/device/protocol/virtual/genericVirtualDeviceLogicFactory.ts index f971252d..4cc69331 100644 --- a/src/device/protocol/virtual/genericVirtualDeviceLogicFactory.ts +++ b/src/device/protocol/virtual/genericVirtualDeviceLogicFactory.ts @@ -1,3 +1,4 @@ +import { TSchema } from '@sinclair/typebox'; import VirtualDeviceLogic from './virtualDeviceLogic.js'; import Logger from '../../../logging/Logger.js'; import VirtualDeviceLogicFactory from './virtualDeviceLogicFactory.js'; @@ -9,25 +10,22 @@ export default class GenericVirtualDeviceLogicFactory< TDeviceLogic extends VirtualDeviceLogic > implements VirtualDeviceLogicFactory { + public readonly configSchema: TSchema & { static: ExtractConfig }; + private readonly ctor: Constructor; private readonly logger: Logger; - private constructor(ctor: Constructor, logger: Logger) { + public constructor( + ctor: Constructor, + configSchema: TSchema & { static: ExtractConfig }, + logger: Logger + ) { this.ctor = ctor; + this.configSchema = configSchema; this.logger = logger; } - public static from>( - genericVirtualDeviceLogicLogicConstructor: Constructor, - logger: Logger - ): GenericVirtualDeviceLogicFactory { - return new GenericVirtualDeviceLogicFactory( - genericVirtualDeviceLogicLogicConstructor, - logger, - ); - } - public create(config: ExtractConfig): TDeviceLogic { return new this.ctor(config, this.logger); } diff --git a/src/device/protocol/virtual/virtualDeviceLogicFactory.ts b/src/device/protocol/virtual/virtualDeviceLogicFactory.ts index 1b002d31..d420645b 100644 --- a/src/device/protocol/virtual/virtualDeviceLogicFactory.ts +++ b/src/device/protocol/virtual/virtualDeviceLogicFactory.ts @@ -1,9 +1,25 @@ +import { TSchema } from '@sinclair/typebox'; import VirtualDeviceLogic from './virtualDeviceLogic.js'; type ExtractConfig> = T extends VirtualDeviceLogic ? C : never; export default interface VirtualDeviceLogicFactory> { + /** + * Known limitation: this only catches a `configSchema` that doesn't match `TDeviceLogic`'s + * own config when that config has at least one *required* property (verified: e.g. pairing + * `PiperVirtualDeviceLogic`, whose config requires `model`, with `noDeviceConfigSchema` is + * correctly rejected). If every property is optional (e.g. `TtsVirtualDeviceConfig`'s `{ + * voice?: string }`), that type is structurally indistinguishable from `{}` under TS's + * assignability rules, so a wrong-but-also-all-optional schema slips through unnoticed at + * compile time (verified: pairing `TtsVirtualDeviceLogic` with `noDeviceConfigSchema` compiles + * without error). Low practical impact: `GenericVirtualDeviceFactory.create()` still validates + * the real config against `configSchema` via AJV at runtime, so a wrong pairing fails loudly + * (a validation error) the first time it's actually exercised, rather than silently + * misbehaving. + */ + readonly configSchema: TSchema & { static: ExtractConfig }; + create(config: ExtractConfig): TDeviceLogic; forDeviceType(): string; diff --git a/src/device/protocol/virtual/virtualDeviceProvider.ts b/src/device/protocol/virtual/virtualDeviceProvider.ts index fbcaa97a..bc106c6a 100644 --- a/src/device/protocol/virtual/virtualDeviceProvider.ts +++ b/src/device/protocol/virtual/virtualDeviceProvider.ts @@ -9,8 +9,9 @@ import VirtualDeviceFactory from './virtualDeviceFactory.js'; import DeviceManager from '../../deviceManager.js'; import { asyncHandler, setImmediateInterval } from '../../../util/async.js'; import { logError } from '../../../util/error.js'; +import { VirtualDeviceProviderConfig } from './virtualDeviceProviderConfig.js'; -export default class VirtualDeviceProvider extends DeviceProvider +export default class VirtualDeviceProvider extends DeviceProvider { public static readonly providerName = 'virtual'; @@ -21,33 +22,33 @@ export default class VirtualDeviceProvider extends DeviceProvider private readonly settingsManager: SettingsManager; - private readonly scanIntervalMs: number; - private discoveryInterval?: NodeJS.Timeout; private stopped: boolean = false; public constructor( + config: VirtualDeviceProviderConfig, deviceManager: DeviceManager, eventEmitter: EventEmitter, deviceFactory: VirtualDeviceFactory, settingsManager: SettingsManager, - logger: Logger, - scanIntervalMs: number + logger: Logger ) { - super(deviceManager, eventEmitter, logger.child({ name: VirtualDeviceProvider.name })); + super(config, deviceManager, eventEmitter, logger.child({ name: VirtualDeviceProvider.name })); this.deviceFactory = deviceFactory; this.settingsManager = settingsManager; - this.scanIntervalMs = scanIntervalMs; } public override async init(): Promise { this.stopped = false; + // `DeviceProviderManager` hydrates missing config fields with their schema `default` + // before validating/constructing, so `scanIntervalMs` is always present here - no + // fallback needed. this.discoveryInterval ??= setImmediateInterval(asyncHandler( this.discoverVirtualDevices.bind(this), (e: unknown) => this.logger.error('Error while scanning for new virtual devices', e) - ), this.scanIntervalMs); + ), this.config.scanIntervalMs); } public override async stop(): Promise { diff --git a/src/device/protocol/virtual/virtualDeviceProviderConfig.ts b/src/device/protocol/virtual/virtualDeviceProviderConfig.ts new file mode 100644 index 00000000..3f332519 --- /dev/null +++ b/src/device/protocol/virtual/virtualDeviceProviderConfig.ts @@ -0,0 +1,15 @@ +import { Type, Static } from '@sinclair/typebox'; + +/** + * `scanIntervalMs` defaults to 3000 when missing - `DeviceProviderManager` hydrates missing + * fields with their schema `default` before validating/constructing, so `VirtualDeviceProvider` + * itself can treat this as always present, without its own fallback logic. + */ +export const virtualDeviceProviderConfigSchema = Type.Object({ + scanIntervalMs: Type.Number({ minimum: 1, default: 3000 }), +}, { + additionalProperties: false, +}); + +export type VirtualDeviceProviderConfigSchema = typeof virtualDeviceProviderConfigSchema; +export type VirtualDeviceProviderConfig = Static; diff --git a/src/device/protocol/virtual/virtualDeviceProviderFactory.ts b/src/device/protocol/virtual/virtualDeviceProviderFactory.ts deleted file mode 100644 index a3e6a419..00000000 --- a/src/device/protocol/virtual/virtualDeviceProviderFactory.ts +++ /dev/null @@ -1,55 +0,0 @@ -import DeviceProviderFactory from '../../provider/deviceProviderFactory.js'; -import Logger from '../../../logging/Logger.js'; -import VirtualDeviceProvider from './virtualDeviceProvider.js'; -import SettingsManager from '../../../settings/settingsManager.js'; -import VirtualDeviceFactory from './virtualDeviceFactory.js'; -import DeviceManager from '../../deviceManager.js'; -import EventEmitterFactory from '../../../factory/eventEmitterFactory.js'; - -type VirtualDeviceProviderConfig = { - scanIntervalMs: number, -} - -export default class VirtualDeviceProviderFactory implements DeviceProviderFactory -{ - private static readonly DEFAULT_SCAN_INTERVAL_MS = 3000; - - private readonly deviceManager: DeviceManager; - - private readonly eventEmitterFactory: EventEmitterFactory; - - private readonly deviceFactory: VirtualDeviceFactory; - - private readonly settingsManager: SettingsManager; - - private readonly logger: Logger; - - public constructor( - deviceManager: DeviceManager, - eventEmitterFactory: EventEmitterFactory, - deviceFactory: VirtualDeviceFactory, - settingsManager: SettingsManager, - logger: Logger - ) { - this.deviceManager = deviceManager; - this.eventEmitterFactory = eventEmitterFactory; - this.deviceFactory = deviceFactory; - this.settingsManager = settingsManager; - this.logger = logger; - } - - public create(config: VirtualDeviceProviderConfig): VirtualDeviceProvider { - const scanIntervalMs = typeof config.scanIntervalMs === 'number' && config.scanIntervalMs > 0 - ? config.scanIntervalMs - : VirtualDeviceProviderFactory.DEFAULT_SCAN_INTERVAL_MS; - - return new VirtualDeviceProvider( - this.deviceManager, - this.eventEmitterFactory.create(), - this.deviceFactory, - this.settingsManager, - this.logger, - scanIntervalMs, - ); - } -} diff --git a/src/device/protocol/zc95/zc95SerialDeviceProvider.ts b/src/device/protocol/zc95/zc95SerialDeviceProvider.ts index 71703ee3..d432b28b 100644 --- a/src/device/protocol/zc95/zc95SerialDeviceProvider.ts +++ b/src/device/protocol/zc95/zc95SerialDeviceProvider.ts @@ -14,8 +14,9 @@ import Zc95MessageFactory from './zc95MessageFactory.js'; import SerialDeviceTransportFactory from '../../transport/serialDeviceTransportFactory.js'; import DeviceManager from '../../deviceManager.js'; import { SerialDeviceInfo } from '../../transport/serialPortObserver.js'; +import { NoDeviceProviderConfig } from '../../provider/deviceProviderConfig.js'; -export default class Zc95SerialDeviceProvider extends SerialDeviceProvider +export default class Zc95SerialDeviceProvider extends SerialDeviceProvider { public static readonly providerName = 'zc95Serial'; @@ -24,6 +25,7 @@ export default class Zc95SerialDeviceProvider extends SerialDeviceProvider, + D extends BleDevice, + TProviderConfig, TAttributes extends DeviceAttributes = InferBleDeviceAttributes, TNotifications extends DeviceNotifications = InferDeviceNotifications, - TConfig extends AnyDeviceConfig = InferBleDeviceConfig -> extends DeviceProvider + TDeviceConfig extends AnyDeviceConfig = InferBleDeviceConfig +> extends DeviceProvider { private connectedDevices: Set = new Set(); - protected constructor(deviceManager: DeviceManager, eventEmitter: EventEmitter, logger: Logger) { - super(deviceManager, eventEmitter, logger); + protected constructor(config: TProviderConfig, deviceManager: DeviceManager, eventEmitter: EventEmitter, logger: Logger) { + super(config, deviceManager, eventEmitter, logger); this.deviceManager.on( DeviceManagerEvent.deviceDetected, diff --git a/src/device/provider/deviceProvider.ts b/src/device/provider/deviceProvider.ts index 5292c4ad..615dab7d 100644 --- a/src/device/provider/deviceProvider.ts +++ b/src/device/provider/deviceProvider.ts @@ -2,7 +2,22 @@ import EventEmitter from 'events'; import Logger from '../../logging/Logger.js'; import DeviceManager from '../deviceManager.js'; -export default abstract class DeviceProvider +/** + * `TConfig` is the provider's own settings.json `DeviceSource.config` shape, named/typed by each + * concrete provider rather than passed around as a loose `JsonObject`. Every provider is paired + * with a TypeBox schema for `TConfig` on its `DeviceProviderFactory`, which validates a + * `DeviceSource`'s raw config against it before constructing the provider. Providers that don't + * need any configuration still use a config type (the shared `NoDeviceProviderConfig`) for + * uniformity - see + * `SlvCtrlPlusSerialDeviceProvider`/`Zc95SerialDeviceProvider`/`EStim2bSerialDeviceProvider`/ + * `AiroticDeviceProvider`. + * + * `config` is deliberately the FIRST constructor parameter across every `DeviceProvider` + * subclass - `GenericDeviceProviderFactory` relies on that fixed convention to generically + * prepend the validated config to whatever other ("dependency") constructor arguments were + * captured when the factory itself was wired up. + */ +export default abstract class DeviceProvider { protected readonly deviceManager: DeviceManager; @@ -10,7 +25,10 @@ export default abstract class DeviceProvider protected readonly logger: Logger; - protected constructor(deviceManager: DeviceManager, eventEmitter: EventEmitter, logger: Logger) { + protected readonly config: TConfig; + + protected constructor(config: TConfig, deviceManager: DeviceManager, eventEmitter: EventEmitter, logger: Logger) { + this.config = config; this.deviceManager = deviceManager; this.eventEmitter = eventEmitter; this.logger = logger; diff --git a/src/device/provider/deviceProviderConfig.ts b/src/device/provider/deviceProviderConfig.ts new file mode 100644 index 00000000..5f16e2e1 --- /dev/null +++ b/src/device/provider/deviceProviderConfig.ts @@ -0,0 +1,12 @@ +import { Type, Static } from '@sinclair/typebox'; + +/** + * Distinct from `NoDeviceConfig` (src/device/deviceConfig.ts) even though it's the same + * underlying shape - that one is for a *device's* own config (e.g. `DisplayVirtualDeviceLogic`), + * this one is for a *provider's* config (e.g. `SlvCtrlPlusSerialDeviceProvider`, which has no + * settings.json `DeviceSource.config` of its own). Kept separate for clarity at call sites, even + * though nothing behaviorally distinguishes the two. + */ +export const noDeviceProviderConfigSchema = Type.Object({}, { additionalProperties: false }); +export type NoDeviceProviderConfigSchema = typeof noDeviceProviderConfigSchema; +export type NoDeviceProviderConfig = Static; diff --git a/src/device/provider/deviceProviderFactory.ts b/src/device/provider/deviceProviderFactory.ts index 4aa6c595..871c1792 100644 --- a/src/device/provider/deviceProviderFactory.ts +++ b/src/device/provider/deviceProviderFactory.ts @@ -1,7 +1,38 @@ +import { TSchema } from '@sinclair/typebox'; import DeviceProvider from './deviceProvider.js'; -import { JsonObject } from '../../types.js'; -export default interface DeviceProviderFactory +/** + * Extracts the `TConfig` a concrete `DeviceProvider` subclass was declared with, so + * `DeviceProviderFactory` only needs a single type parameter instead of repeating the config + * type separately. + */ +export type ConfigOf = DP extends DeviceProvider ? TConfig : never; + +/** + * Constructs one `DeviceProvider` instance for a single `DeviceSource` config entry. + * + * `configSchema` is the TypeBox schema for `ConfigOf` - `DeviceProviderManager` validates a + * `DeviceSource`'s raw config against it before calling `create()`, so factories don't need to + * defensively parse raw JSON themselves. TypeBox schemas carry their inferred type as a phantom + * `static` property, so intersecting it with `{ static: ConfigOf }` is enough for TS to catch + * a factory whose schema doesn't actually match its own `DP`'s config. + * + * Bounded to `TSchema` rather than `TObject`: `TObject` is itself generic with a recursive + * default type parameter, and using it bare as a field type (rather than only ever as a generic + * parameter bound, substituted with a concrete narrow type) blows TS's instantiation depth limit. + * `TSchema` is a plain, non-generic marker interface, so it doesn't have this problem. + * + * Known limitation: this only catches a mismatched `configSchema` when `ConfigOf` has at + * least one *required* property. If every property is optional (e.g. `{ scanIntervalMs?: number + * }`), that type is structurally indistinguishable from `{}` under TS's assignability rules (each + * is assignable to the other), so a wrong-but-also-all-optional schema slips through unnoticed at + * compile time. Low practical impact: `DeviceProviderManager` still validates the real config + * against `configSchema` via AJV at runtime, so a wrong pairing fails loudly (a validation error) + * the first time it's actually exercised, rather than silently misbehaving. + */ +export default interface DeviceProviderFactory> { - create(config: JsonObject): DP; + readonly configSchema: TSchema & { static: ConfigOf }; + + create(config: ConfigOf): DP; } diff --git a/src/device/provider/deviceProviderManager.ts b/src/device/provider/deviceProviderManager.ts index 1620a8e1..daaf5dc0 100644 --- a/src/device/provider/deviceProviderManager.ts +++ b/src/device/provider/deviceProviderManager.ts @@ -1,21 +1,27 @@ +import { Value } from '@sinclair/typebox/value'; import Settings from '../../settings/settings.js'; import DeviceProviderFactory from './deviceProviderFactory.js'; import Logger from '../../logging/Logger.js'; import DeviceProvider from './deviceProvider.js'; +import JsonSchemaValidatorFactory from '../../schemaValidation/JsonSchemaValidatorFactory.js'; export default class DeviceProviderManager { private factories: Map>; + private readonly jsonSchemaValidatorFactory: JsonSchemaValidatorFactory; + private readonly logger: Logger; - private providers: DeviceProvider[] = []; + private providers: DeviceProvider[] = []; public constructor( factories: Map>, + jsonSchemaValidatorFactory: JsonSchemaValidatorFactory, logger: Logger ) { this.factories = factories; + this.jsonSchemaValidatorFactory = jsonSchemaValidatorFactory; this.logger = logger.child({ name: DeviceProviderManager.name }); } @@ -33,7 +39,26 @@ export default class DeviceProviderManager continue; } - const provider = factory.create(deviceSource.config); + // Clone before hydrating: `Value.Default()` mutates in place, and we don't want to + // write resolved defaults back into `deviceSource.config` itself (Settings auto-saves + // on mutation, so that would trigger a spurious settings.json write/broadcast). + // `structuredClone()` doesn't work here: `Settings` is wrapped in an `on-change` Proxy + // that deep-proxies nested objects too (including `deviceSource.config`), and the + // structured clone algorithm can't clone a Proxy. `JsonObject` is JSON-safe by + // definition, so a plain JSON round-trip clones it fine while transparently reading + // through the proxy (JSON.stringify just does normal property access). + const config = Value.Default(factory.configSchema, JSON.parse(JSON.stringify(deviceSource.config))); + + const configValidator = this.jsonSchemaValidatorFactory.create(factory.configSchema); + + if (!configValidator.validate(config)) { + throw new Error( + `Config for device source '${id}' (type '${deviceSource.type}') is not valid: ` + + configValidator.getValidationErrorsAsText() + ); + } + + const provider = factory.create(config); this.providers.push(provider); } diff --git a/src/device/provider/genericDeviceProviderFactory.ts b/src/device/provider/genericDeviceProviderFactory.ts index abcfb9f6..ac77c8ee 100644 --- a/src/device/provider/genericDeviceProviderFactory.ts +++ b/src/device/provider/genericDeviceProviderFactory.ts @@ -1,21 +1,36 @@ +import { TSchema } from '@sinclair/typebox'; import DeviceProvider from './deviceProvider.js'; -import DeviceProviderFactory from './deviceProviderFactory.js'; - -type ConcreteCtor = new (...args: any[]) => T; +import DeviceProviderFactory, { ConfigOf } from './deviceProviderFactory.js'; +/** + * Every `DeviceProvider` constructor starts with `config: ConfigOf` as its first parameter + * (see `DeviceProvider`'s own doc comment) - this is what lets `GenericDeviceProviderFactory` + * capture every other constructor argument once, at DI-wiring time (`TDependencyArgs` - the + * "always the same, regardless of `DeviceSource`" deps, as opposed to `config`, which varies per + * `DeviceSource`), and prepend the actual validated config later, once per `DeviceSource`, in + * `create()`. + */ export default class GenericDeviceProviderFactory< - DP extends DeviceProvider + DP extends DeviceProvider, + TDependencyArgs extends any[] = any[] > implements DeviceProviderFactory { - private readonly ctor: ConcreteCtor; - private readonly args: ConstructorParameters>; + public readonly configSchema: TSchema & { static: ConfigOf }; + + private readonly ctor: new (config: ConfigOf, ...dependencyArgs: TDependencyArgs) => DP; + private readonly dependencyArgs: TDependencyArgs; - public constructor(ctor: ConcreteCtor, ...args: ConstructorParameters>) { + public constructor( + configSchema: TSchema & { static: ConfigOf }, + ctor: new (config: ConfigOf, ...dependencyArgs: TDependencyArgs) => DP, + ...dependencyArgs: TDependencyArgs + ) { + this.configSchema = configSchema; this.ctor = ctor; - this.args = args; + this.dependencyArgs = dependencyArgs; } - public create(): DP { - return new this.ctor(...this.args); + public create(config: ConfigOf): DP { + return new this.ctor(config, ...this.dependencyArgs); } } diff --git a/src/device/provider/serialDeviceProvider.ts b/src/device/provider/serialDeviceProvider.ts index c3d0769e..51c5221d 100644 --- a/src/device/provider/serialDeviceProvider.ts +++ b/src/device/provider/serialDeviceProvider.ts @@ -18,10 +18,11 @@ import { AnyDeviceConfig } from '../deviceConfig.js'; export type SerialDeviceProviderPortOpenOptions = Omit, 'path' | 'autoOpen'>; export default abstract class SerialDeviceProvider< - D extends PeripheralDevice, + D extends PeripheralDevice, + TProviderConfig, TAttributes extends DeviceAttributes = InferPeripheralDeviceAttributes, - TConfig extends AnyDeviceConfig = InferPeripheralDeviceConfig -> extends DeviceProvider + TDeviceConfig extends AnyDeviceConfig = InferPeripheralDeviceConfig +> extends DeviceProvider { private readonly serialPortFactory: SerialPortFactory; @@ -29,8 +30,14 @@ export default abstract class SerialDeviceProvider< private readonly deviceDetectedListener: (deviceInfo: DeviceInfo) => void; - protected constructor(deviceManager: DeviceManager, serialPortFactory: SerialPortFactory, eventEmitter: EventEmitter, logger: Logger) { - super(deviceManager, eventEmitter, logger); + protected constructor( + config: TProviderConfig, + deviceManager: DeviceManager, + serialPortFactory: SerialPortFactory, + eventEmitter: EventEmitter, + logger: Logger + ) { + super(config, deviceManager, eventEmitter, logger); this.serialPortFactory = serialPortFactory; diff --git a/src/serviceMap.ts b/src/serviceMap.ts index e27d557c..c4ebe319 100644 --- a/src/serviceMap.ts +++ b/src/serviceMap.ts @@ -32,7 +32,6 @@ import RunScriptController from './controller/automation/runScriptController.js' import StopScriptController from './controller/automation/stopScriptController.js'; import StatusScriptController from './controller/automation/statusScriptController.js'; import VirtualDeviceProvider from './device/protocol/virtual/virtualDeviceProvider.js'; -import VirtualDeviceProviderFactory from './device/protocol/virtual/virtualDeviceProviderFactory.js'; import GetSettingsController from './controller/settings/getSettingsController.js'; import PutSettingsController from './controller/settings/putSettingsController.js'; import JsonSchemaValidatorFactory from './schemaValidation/JsonSchemaValidatorFactory.js'; @@ -50,6 +49,7 @@ import Zc95SerialDeviceProvider from './device/protocol/zc95/zc95SerialDevicePro import EStim2bSerialDeviceProvider from './device/protocol/estim2b/estim2bSerialDeviceProvider.js'; import ButtplugIoWebsocketDeviceProvider from './device/protocol/buttplugIo/buttplugIoWebsocketDeviceProvider.js'; import AiroticDeviceProvider from './device/protocol/airotic/airoticDeviceProvider.js'; +import KnownDeviceRegistry from './device/knownDeviceRegistry.js'; type ServiceMap = { @@ -63,7 +63,7 @@ type ServiceMap = { /* deviceServiceProvider */ 'device.manager': DeviceManager, 'device.serial.transport.factory': SerialDeviceTransportFactory, - 'device.provider.factory.virtual': VirtualDeviceProviderFactory, + 'device.provider.factory.virtual': DeviceProviderFactory, 'device.serial.factory.slvCtrlPlus': SlvCtrlPlusDeviceFactory, 'device.factory.zc95': Zc95DeviceFactory, 'device.factory.estim2b': Estim2bDeviceFactory, @@ -76,6 +76,7 @@ type ServiceMap = { 'device.virtual.provider': VirtualDeviceProvider, 'device.virtual.factory': VirtualDeviceFactory, 'device.uniqueNameGenerator': DeviceNameGenerator, + 'device.knownDeviceRegistry': KnownDeviceRegistry, 'device.updater': DeviceUpdaterInterface, 'device.observer.serial': SerialPortObserver, 'device.observer.ble': BleObserver, diff --git a/src/serviceProvider/deviceServiceProvider.ts b/src/serviceProvider/deviceServiceProvider.ts index ffe303ac..7177a06a 100644 --- a/src/serviceProvider/deviceServiceProvider.ts +++ b/src/serviceProvider/deviceServiceProvider.ts @@ -11,12 +11,11 @@ import Device from '../device/device.js'; import DeviceProviderManager from '../device/provider/deviceProviderManager.js'; import SlvCtrlPlusSerialDeviceProvider from '../device/protocol/slvCtrlPlus/slvCtrlPlusSerialDeviceProvider.js'; import ButtplugIoWebsocketDeviceProvider from '../device/protocol/buttplugIo/buttplugIoWebsocketDeviceProvider.js'; -import ButtplugIoWebsocketDeviceProviderFactory - from '../device/protocol/buttplugIo/buttplugIoWebsocketDeviceProviderFactory.js'; +import { buttplugIoWebsocketConfigSchema } from '../device/protocol/buttplugIo/buttplugIoWebsocketConfig.js'; import ButtplugIoDeviceFactory from '../device/protocol/buttplugIo/buttplugIoDeviceFactory.js'; import ServiceMap from '../serviceMap.js'; import VirtualDeviceProvider from '../device/protocol/virtual/virtualDeviceProvider.js'; -import VirtualDeviceProviderFactory from '../device/protocol/virtual/virtualDeviceProviderFactory.js'; +import { virtualDeviceProviderConfigSchema } from '../device/protocol/virtual/virtualDeviceProviderConfig.js'; import GenericVirtualDeviceFactory from '../device/protocol/virtual/genericVirtualDeviceFactory.js'; import DisplayVirtualDeviceLogic from '../device/protocol/virtual/display/displayVirtualDeviceLogic.js'; import RandomGeneratorVirtualDeviceLogic @@ -28,6 +27,7 @@ import Zc95DeviceFactory from '../device/protocol/zc95/zc95DeviceFactory.js'; import PiperVirtualDeviceLogic from '../device/protocol/virtual/audio/piperVirtualDeviceLogic.js'; import { piperVirtualDeviceConfigSchema } from '../device/protocol/virtual/audio/piperVirtualDeviceConfig.js'; import { noDeviceConfigSchema } from '../device/deviceConfig.js'; +import { noDeviceProviderConfigSchema } from '../device/provider/deviceProviderConfig.js'; import { randomGeneratorVirtualDeviceConfigSchema } from '../device/protocol/virtual/randomGenerator/randomGeneratorVirtualDeviceConfig.js'; @@ -40,6 +40,7 @@ import BleObserver from '../device/transport/bleObserver.js'; import AiroticDeviceProvider from '../device/protocol/airotic/airoticDeviceProvider.js'; import DeviceProviderFactory from '../device/provider/deviceProviderFactory.js'; import { DeviceId } from '../device/deviceId.js'; +import KnownDeviceRegistry from '../device/knownDeviceRegistry.js'; export default class DeviceServiceProvider implements ServiceProvider { public register(container: Pimple): void { @@ -51,6 +52,7 @@ export default class DeviceServiceProvider implements ServiceProvider new GenericDeviceProviderFactory( + noDeviceProviderConfigSchema, SlvCtrlPlusSerialDeviceProvider, container.get('device.manager'), container.get('factory.serialPort'), @@ -63,7 +65,9 @@ export default class DeviceServiceProvider implements ServiceProvider new ButtplugIoWebsocketDeviceProviderFactory( + () => new GenericDeviceProviderFactory( + buttplugIoWebsocketConfigSchema, + ButtplugIoWebsocketDeviceProvider, container.get('device.manager'), container.get('factory.eventEmitter').create(), container.get('device.serial.factory.buttplugIo'), @@ -90,18 +94,23 @@ export default class DeviceServiceProvider implements ServiceProvider new KnownDeviceRegistry( + container.get('settings'), + container.get('device.uniqueNameGenerator'), + container.get('logger.default'), + )); + container.set('device.serial.factory.slvCtrlPlus', () => new SlvCtrlPlusDeviceFactory( container.get('factory.date'), container.get('factory.eventEmitter'), - container.get('settings'), - container.get('device.uniqueNameGenerator'), + container.get('device.knownDeviceRegistry'), container.get('logger.default'), )); container.set('device.serial.factory.buttplugIo', () => new ButtplugIoDeviceFactory( container.get('factory.date'), container.get('factory.eventEmitter'), - container.get('settings'), + container.get('device.knownDeviceRegistry'), container.get('logger.default'), )); @@ -121,9 +130,11 @@ export default class DeviceServiceProvider implements ServiceProvider new VirtualDeviceProviderFactory( + container.set('device.provider.factory.virtual', () => new GenericDeviceProviderFactory( + virtualDeviceProviderConfigSchema, + VirtualDeviceProvider, container.get('device.manager'), - container.get('factory.eventEmitter'), + container.get('factory.eventEmitter').create(), container.get('device.virtual.factory'), container.get('settings.manager'), container.get('logger.default'), @@ -140,22 +151,26 @@ export default class DeviceServiceProvider implements ServiceProvider { return new GenericDeviceProviderFactory( + noDeviceProviderConfigSchema, Zc95SerialDeviceProvider, container.get('device.manager'), container.get('factory.serialPort'), @@ -215,6 +232,7 @@ export default class DeviceServiceProvider implements ServiceProvider { return new GenericDeviceProviderFactory( + noDeviceProviderConfigSchema, EStim2bSerialDeviceProvider, container.get('device.manager'), container.get('factory.serialPort'), @@ -227,9 +245,10 @@ export default class DeviceServiceProvider implements ServiceProvider { return new GenericDeviceProviderFactory( + noDeviceProviderConfigSchema, AiroticDeviceProvider, container.get('device.manager'), - container.get('settings'), + container.get('device.knownDeviceRegistry'), container.get('factory.eventEmitter').create(), container.get('logger.default'), ); diff --git a/tests/unit/device/knownDeviceRegistry.spec.ts b/tests/unit/device/knownDeviceRegistry.spec.ts new file mode 100644 index 00000000..6658fea7 --- /dev/null +++ b/tests/unit/device/knownDeviceRegistry.spec.ts @@ -0,0 +1,120 @@ +import { beforeEach, describe, expect, it } from 'vitest'; +import { mock } from 'vitest-mock-extended'; +import KnownDeviceRegistry from '../../../src/device/knownDeviceRegistry.js'; +import Settings from '../../../src/settings/settings.js'; +import DeviceNameGenerator from '../../../src/device/deviceNameGenerator.js'; +import Logger from '../../../src/logging/Logger.js'; +import KnownDevice from '../../../src/settings/knownDevice.js'; +import { DeviceId } from '../../../src/device/deviceId.js'; + +describe('KnownDeviceRegistry', () => { + let mockSettings: ReturnType>; + let mockNameGenerator: ReturnType>; + let mockLogger: ReturnType>; + let registry: KnownDeviceRegistry; + + beforeEach(() => { + mockSettings = mock(); + mockNameGenerator = mock(); + mockNameGenerator.generateName.mockReturnValue('Generated Name'); + mockLogger = mock(); + mockLogger.child.mockReturnValue(mockLogger); + + registry = new KnownDeviceRegistry(mockSettings, mockNameGenerator, mockLogger); + }); + + describe('resolve', () => { + it('returns the already known device without persisting anything', () => { + const existingKnownDevice = new KnownDevice(DeviceId.create('device-1'), 'Existing Name', 'testType', 'testProvider'); + mockSettings.getKnownDeviceById.mockReturnValue(existingKnownDevice); + + const result = registry.resolve(DeviceId.create('device-1'), 'testType', 'testProvider'); + + expect(result).toBe(existingKnownDevice); + expect(mockSettings.addKnownDevice).not.toHaveBeenCalled(); + }); + + it('builds a new, not-yet-persisted KnownDevice when none exists', () => { + mockSettings.getKnownDeviceById.mockReturnValue(undefined); + + const deviceId = DeviceId.create('device-1'); + const result = registry.resolve(deviceId, 'testType', 'testProvider'); + + expect(result).toMatchObject({ id: deviceId, type: 'testType', source: 'testProvider' }); + expect(mockSettings.addKnownDevice).not.toHaveBeenCalled(); + }); + + it('uses the provided name over the generated one for a new device', () => { + mockSettings.getKnownDeviceById.mockReturnValue(undefined); + + const result = registry.resolve(DeviceId.create('device-1'), 'testType', 'testProvider', 'Explicit Name'); + + expect(result.name).toBe('Explicit Name'); + expect(mockNameGenerator.generateName).not.toHaveBeenCalled(); + }); + + it('falls back to a generated name when none is provided', () => { + mockSettings.getKnownDeviceById.mockReturnValue(undefined); + + const result = registry.resolve(DeviceId.create('device-1'), 'testType', 'testProvider'); + + expect(result.name).toBe('Generated Name'); + }); + }); + + describe('persist', () => { + it('delegates to settings.addKnownDevice for a genuinely new identity', () => { + const knownDevice = new KnownDevice(DeviceId.create('device-1'), 'Name', 'testType', 'testProvider'); + mockSettings.getKnownDeviceById.mockReturnValue(undefined); + + registry.persist(knownDevice); + + expect(mockSettings.addKnownDevice).toHaveBeenCalledOnce(); + expect(mockSettings.addKnownDevice).toHaveBeenCalledWith(knownDevice); + }); + + it('does not touch settings when persisting an already-known, unchanged identity', () => { + // This matters beyond avoiding pointless work: Settings is wrapped with on-change to + // auto-save to disk, so calling addKnownDevice() here unconditionally would trigger a + // settings.json write + a settings-changed broadcast on every device (re)connect, even + // for a device that's been known and unchanged for months. + const existingKnownDevice = new KnownDevice(DeviceId.create('device-1'), 'Name', 'testType', 'testProvider'); + mockSettings.getKnownDeviceById.mockReturnValue(existingKnownDevice); + + registry.persist(existingKnownDevice); + + expect(mockSettings.addKnownDevice).not.toHaveBeenCalled(); + }); + + it('persists when passed a different KnownDevice instance for an already-known id', () => { + const existingKnownDevice = new KnownDevice(DeviceId.create('device-1'), 'Name', 'testType', 'testProvider'); + const differentInstance = new KnownDevice(DeviceId.create('device-1'), 'Name', 'testType', 'testProvider'); + mockSettings.getKnownDeviceById.mockReturnValue(existingKnownDevice); + + registry.persist(differentInstance); + + expect(mockSettings.addKnownDevice).toHaveBeenCalledWith(differentInstance); + }); + }); + + describe('resolve + persist integration', () => { + it('does not write to settings when reconnecting an already-known device', () => { + const existingKnownDevice = new KnownDevice(DeviceId.create('device-1'), 'Existing Name', 'testType', 'testProvider'); + mockSettings.getKnownDeviceById.mockReturnValue(existingKnownDevice); + + const knownDevice = registry.resolve(DeviceId.create('device-1'), 'testType', 'testProvider'); + registry.persist(knownDevice); + + expect(mockSettings.addKnownDevice).not.toHaveBeenCalled(); + }); + + it('writes to settings exactly once when connecting a genuinely new device', () => { + mockSettings.getKnownDeviceById.mockReturnValue(undefined); + + const knownDevice = registry.resolve(DeviceId.create('device-1'), 'testType', 'testProvider'); + registry.persist(knownDevice); + + expect(mockSettings.addKnownDevice).toHaveBeenCalledOnce(); + }); + }); +}); diff --git a/tests/unit/device/provider/deviceProviderManager.spec.ts b/tests/unit/device/provider/deviceProviderManager.spec.ts new file mode 100644 index 00000000..20b70670 --- /dev/null +++ b/tests/unit/device/provider/deviceProviderManager.spec.ts @@ -0,0 +1,225 @@ +import EventEmitter from 'events'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { mock } from 'vitest-mock-extended'; +import { Type } from '@sinclair/typebox'; +import { Ajv2020 } from 'ajv/dist/2020.js'; +import ajvFormatsPlugin from 'ajv-formats'; +import DeviceProviderManager from '../../../../src/device/provider/deviceProviderManager.js'; +import DeviceProviderFactory from '../../../../src/device/provider/deviceProviderFactory.js'; +import DeviceProvider from '../../../../src/device/provider/deviceProvider.js'; +import JsonSchemaValidatorFactory from '../../../../src/schemaValidation/JsonSchemaValidatorFactory.js'; +import Settings from '../../../../src/settings/settings.js'; +import DeviceSource from '../../../../src/settings/deviceSource.js'; +import Logger from '../../../../src/logging/Logger.js'; +import DeviceManager from '../../../../src/device/deviceManager.js'; + +type FakeProviderConfig = { greeting: string }; + +const fakeProviderConfigSchema = Type.Object({ + greeting: Type.String(), +}, { additionalProperties: false }); + +type FakeDefaultedProviderConfig = { retries: number }; + +const fakeDefaultedProviderConfigSchema = Type.Object({ + retries: Type.Number({ default: 3 }), +}, { additionalProperties: false }); + +class FakeDeviceProvider extends DeviceProvider { + public readonly initMock = vi.fn().mockResolvedValue(undefined); + public readonly stopMock = vi.fn().mockResolvedValue(undefined); + + public constructor(logger: Logger, config: FakeProviderConfig) { + super(config, mock(), new EventEmitter(), logger); + } + + public override init(): Promise { + return this.initMock(); + } + + public override stop(): Promise { + return this.stopMock(); + } +} + +class FakeDeviceProviderFactory implements DeviceProviderFactory { + public readonly configSchema = fakeProviderConfigSchema; + + public readonly created: FakeDeviceProvider[] = []; + + public constructor(private readonly logger: Logger) { + } + + public create(config: FakeProviderConfig): FakeDeviceProvider { + const provider = new FakeDeviceProvider(this.logger, config); + this.created.push(provider); + return provider; + } +} + +class FakeDefaultedDeviceProvider extends DeviceProvider { + public constructor(logger: Logger, config: FakeDefaultedProviderConfig) { + super(config, mock(), new EventEmitter(), logger); + } +} + +class FakeDefaultedDeviceProviderFactory implements DeviceProviderFactory { + public readonly configSchema = fakeDefaultedProviderConfigSchema; + + public readonly created: FakeDefaultedDeviceProvider[] = []; + + public constructor(private readonly logger: Logger) { + } + + public create(config: FakeDefaultedProviderConfig): FakeDefaultedDeviceProvider { + const provider = new FakeDefaultedDeviceProvider(this.logger, config); + this.created.push(provider); + return provider; + } +} + +describe('DeviceProviderManager', () => { + let mockSettings: ReturnType>; + let mockLogger: ReturnType>; + let jsonSchemaValidatorFactory: JsonSchemaValidatorFactory; + let fakeFactory: FakeDeviceProviderFactory; + let fakeDefaultedFactory: FakeDefaultedDeviceProviderFactory; + let manager: DeviceProviderManager; + + beforeEach(() => { + mockSettings = mock(); + mockLogger = mock(); + mockLogger.child.mockReturnValue(mockLogger); + + const ajv = new Ajv2020({ allErrors: true, strict: true }); + ajvFormatsPlugin.default(ajv); + jsonSchemaValidatorFactory = new JsonSchemaValidatorFactory(ajv); + + fakeFactory = new FakeDeviceProviderFactory(mockLogger); + fakeDefaultedFactory = new FakeDefaultedDeviceProviderFactory(mockLogger); + + manager = new DeviceProviderManager( + new Map>([ + ['fake', fakeFactory], + ['fakeDefaulted', fakeDefaultedFactory], + ]), + jsonSchemaValidatorFactory, + mockLogger, + ); + }); + + describe('loadFromSettings', () => { + it('constructs one provider instance per matching device source, even for the same type', () => { + mockSettings.getDeviceSources.mockReturnValue(new Map([ + ['source-1', new DeviceSource('source-1', 'fake', { greeting: 'hi' })], + ['source-2', new DeviceSource('source-2', 'fake', { greeting: 'hello' })], + ])); + + manager.loadFromSettings(mockSettings); + + expect(fakeFactory.created).toHaveLength(2); + }); + + it('skips a device source whose type has no registered factory, logging a warning', () => { + mockSettings.getDeviceSources.mockReturnValue(new Map([ + ['source-1', new DeviceSource('source-1', 'unsupportedType', {})], + ])); + + manager.loadFromSettings(mockSettings); + + expect(fakeFactory.created).toHaveLength(0); + expect(mockLogger.warn).toHaveBeenCalledWith(expect.stringContaining('unsupportedType')); + }); + + it('throws when a device source config fails schema validation (wrong type)', () => { + mockSettings.getDeviceSources.mockReturnValue(new Map([ + ['source-1', new DeviceSource('source-1', 'fake', { greeting: 123 })], + ])); + + expect(() => manager.loadFromSettings(mockSettings)).toThrow(/not valid/); + expect(fakeFactory.created).toHaveLength(0); + }); + + it('throws when a device source config is missing a required field', () => { + mockSettings.getDeviceSources.mockReturnValue(new Map([ + ['source-1', new DeviceSource('source-1', 'fake', {})], + ])); + + expect(() => manager.loadFromSettings(mockSettings)).toThrow(/not valid/); + }); + + it('throws when a device source config has additional, unknown properties', () => { + mockSettings.getDeviceSources.mockReturnValue(new Map([ + ['source-1', new DeviceSource('source-1', 'fake', { greeting: 'hi', extra: true })], + ])); + + expect(() => manager.loadFromSettings(mockSettings)).toThrow(/not valid/); + }); + + it('hydrates a missing config field with its schema default before validating/constructing', () => { + mockSettings.getDeviceSources.mockReturnValue(new Map([ + ['source-1', new DeviceSource('source-1', 'fakeDefaulted', {})], + ])); + + manager.loadFromSettings(mockSettings); + + expect(fakeDefaultedFactory.created).toHaveLength(1); + expect(fakeDefaultedFactory.created[0]).toMatchObject({ config: { retries: 3 } }); + }); + + it('does not mutate the DeviceSource.config object itself while hydrating defaults', () => { + const rawConfig = {}; + mockSettings.getDeviceSources.mockReturnValue(new Map([ + ['source-1', new DeviceSource('source-1', 'fakeDefaulted', rawConfig)], + ])); + + manager.loadFromSettings(mockSettings); + + expect(rawConfig).toEqual({}); + }); + + it('keeps an explicitly provided value over the schema default', () => { + mockSettings.getDeviceSources.mockReturnValue(new Map([ + ['source-1', new DeviceSource('source-1', 'fakeDefaulted', { retries: 7 })], + ])); + + manager.loadFromSettings(mockSettings); + + expect(fakeDefaultedFactory.created[0]).toMatchObject({ config: { retries: 7 } }); + }); + }); + + describe('startProviders / stopProviders', () => { + beforeEach(() => { + mockSettings.getDeviceSources.mockReturnValue(new Map([ + ['source-1', new DeviceSource('source-1', 'fake', { greeting: 'hi' })], + ['source-2', new DeviceSource('source-2', 'fake', { greeting: 'hello' })], + ])); + manager.loadFromSettings(mockSettings); + }); + + it('initializes every constructed provider', async () => { + await manager.startProviders(); + + for (const provider of fakeFactory.created) { + expect(provider.initMock).toHaveBeenCalledOnce(); + } + }); + + it('stops every provider', async () => { + await manager.stopProviders(); + + for (const provider of fakeFactory.created) { + expect(provider.stopMock).toHaveBeenCalledOnce(); + } + }); + + it('collects errors from failing providers and still stops the rest, then throws', async () => { + fakeFactory.created[0].stopMock.mockRejectedValueOnce(new Error('boom')); + + await expect(manager.stopProviders()).rejects.toThrow(/Failed to stop 1 device provider/); + + expect(fakeFactory.created[1].stopMock).toHaveBeenCalledOnce(); + }); + }); +}); diff --git a/tests/unit/device/testDeviceProvider.ts b/tests/unit/device/testDeviceProvider.ts index 712baae1..51f581be 100644 --- a/tests/unit/device/testDeviceProvider.ts +++ b/tests/unit/device/testDeviceProvider.ts @@ -2,12 +2,13 @@ import {EventEmitter} from "events"; import DeviceProvider from "../../../src/device/provider/deviceProvider.js"; import Logger from "../../../src/logging/Logger.js"; import DeviceManager from "../../../src/device/deviceManager.js"; +import { NoDeviceProviderConfig } from "../../../src/device/provider/deviceProviderConfig.js"; -export default class TestDeviceProvider extends DeviceProvider +export default class TestDeviceProvider extends DeviceProvider { - public constructor(deviceManager: DeviceManager, eventEmitter: EventEmitter, logger: Logger) + public constructor(config: NoDeviceProviderConfig, deviceManager: DeviceManager, eventEmitter: EventEmitter, logger: Logger) { - super(deviceManager, eventEmitter, logger); + super(config, deviceManager, eventEmitter, logger); } public override init(): Promise