fix(configManager): deep-merge domainConfig so general.id and asset.model survive
buildConfig() was doing Object.assign(config, domainConfig) — a shallow
merge. When buildDomainConfig returned subsets like {general: {unit}} or
{asset: {curveUnits}}, the assign replaced config.general / config.asset
wholesale, wiping general.id (nodeId), general.name, asset.model, and
asset.supplier that buildConfig had just populated.
Downstream consequences:
- MGC.onRegister('machine') keys by config.general.id; two children with
the same null id collide and the second registration is rejected.
- rotatingMachine curve-lookup uses asset.model; with model "Unknown" the
pump curve fails to resolve.
Replace Object.assign with an in-place recursive _deepMerge: arrays and
primitives in src replace dst, plain objects merge key-by-key so siblings
on dst survive.
Two regression tests added — single-config field survival and the MGC
multi-child id-collision case.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -130,12 +130,41 @@ class ConfigManager {
|
|||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
// Merge domain-specific sections
|
// Merge domain-specific sections. Must be a DEEP merge: domainConfig
|
||||||
Object.assign(config, 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;
|
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
|
* Migrate a config object from one version to another by applying
|
||||||
* registered migration functions in sequence.
|
* registered migration functions in sequence.
|
||||||
|
|||||||
@@ -3,6 +3,48 @@ const assert = require('node:assert/strict');
|
|||||||
|
|
||||||
const ConfigManager = require('../src/configs/index.js');
|
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', () => {
|
test('can read known config and report existence', () => {
|
||||||
const manager = new ConfigManager('.');
|
const manager = new ConfigManager('.');
|
||||||
assert.equal(manager.hasConfig('measurement'), true);
|
assert.equal(manager.hasConfig('measurement'), true);
|
||||||
|
|||||||
Reference in New Issue
Block a user