diff --git a/src/configs/index.js b/src/configs/index.js index c34fada..ed879ca 100644 --- a/src/configs/index.js +++ b/src/configs/index.js @@ -130,12 +130,41 @@ class ConfigManager { }; } - // Merge domain-specific sections - Object.assign(config, domainConfig); + // Merge domain-specific sections. Must be a DEEP merge: domainConfig + // commonly returns subsets of `general` / `asset` (e.g. {general: + // {unit}}, {asset: {curveUnits}}) and a shallow assign would wipe out + // sibling keys this method just populated — notably `general.id` + // (nodeId) and `asset.model`, causing child-registration id collisions + // and curve-lookup failures downstream. + ConfigManager._deepMerge(config, domainConfig); return config; } + static _isPlainObject(v) { + return Object.prototype.toString.call(v) === '[object Object]'; + } + + /** + * In-place recursive merge. Arrays and primitives in `src` replace `dst`; + * plain objects are merged key-by-key so siblings on `dst` survive. + */ + static _deepMerge(dst, src) { + if (!ConfigManager._isPlainObject(src)) return dst; + for (const key of Object.keys(src)) { + const v = src[key]; + if (Array.isArray(v)) { + dst[key] = [...v]; + } else if (ConfigManager._isPlainObject(v)) { + if (!ConfigManager._isPlainObject(dst[key])) dst[key] = {}; + ConfigManager._deepMerge(dst[key], v); + } else { + dst[key] = v; + } + } + return dst; + } + /** * Migrate a config object from one version to another by applying * registered migration functions in sequence. diff --git a/test/config-manager.test.js b/test/config-manager.test.js index e7147a1..3ac5e43 100644 --- a/test/config-manager.test.js +++ b/test/config-manager.test.js @@ -3,6 +3,48 @@ const assert = require('node:assert/strict'); const ConfigManager = require('../src/configs/index.js'); +test('buildConfig deep-merges domainConfig so general.id and asset.model survive', () => { + // Regression: previously this used Object.assign (shallow), so a + // buildDomainConfig that returned {general:{unit:'m3/h'}, asset:{curveUnits:...}} + // wiped general.id (→ null via schema default) and asset.model (→ "Unknown"), + // which collapsed MGC child registration (id collision) and broke curve loading. + const manager = new ConfigManager('.'); + const uiConfig = { + name: 'PumpA', + model: 'hidrostal-H05K-S03R', + supplier: 'Hidrostal', + category: 'pump', + assetType: 'Centrifugal', + unit: 'm3/h', + }; + const domainConfig = { + general: { unit: 'm3/h' }, + asset: { curveUnits: { pressure: 'mbar', flow: 'm3/h', power: 'kW', control: '%' } }, + }; + const cfg = manager.buildConfig('rotatingMachine', uiConfig, 'node-abc-123', domainConfig); + + assert.equal(cfg.general.id, 'node-abc-123', 'general.id must come from nodeId'); + assert.equal(cfg.general.name, 'PumpA', 'general.name must come from uiConfig.name'); + assert.equal(cfg.general.unit, 'm3/h', 'general.unit must come from domainConfig'); + assert.equal(cfg.asset.model, 'hidrostal-H05K-S03R', 'asset.model must come from uiConfig.model'); + assert.equal(cfg.asset.supplier, 'Hidrostal', 'asset.supplier must survive merge'); + assert.deepEqual(cfg.asset.curveUnits, domainConfig.asset.curveUnits, + 'asset.curveUnits must come from domainConfig'); +}); + +test('buildConfig: two nodes with same uiConfig keep distinct general.id', () => { + // Regression: MGC.onRegister('machine') keys by config.general.id; if two + // machines collapse to the same id (e.g. null), the second is rejected. + const manager = new ConfigManager('.'); + const ui = { model: 'hidrostal-H05K-S03R', supplier: 'Hidrostal', unit: 'm3/h' }; + const dom = { general: { unit: 'm3/h' }, asset: { curveUnits: { pressure: 'mbar' } } }; + const a = manager.buildConfig('rotatingMachine', ui, 'red-node-A', dom); + const b = manager.buildConfig('rotatingMachine', ui, 'red-node-B', dom); + assert.notEqual(a.general.id, b.general.id); + assert.equal(a.general.id, 'red-node-A'); + assert.equal(b.general.id, 'red-node-B'); +}); + test('can read known config and report existence', () => { const manager = new ConfigManager('.'); assert.equal(manager.hasConfig('measurement'), true);