Skip to content

Commit 87478c2

Browse files
noraebnoraeb
committed
fix: avoid data loss when updating LNodeType
1 parent daade82 commit 87478c2

4 files changed

Lines changed: 188 additions & 30 deletions

File tree

‎foundation/utils.spec.ts‎

Lines changed: 81 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
/* eslint-disable no-unused-expressions */
22
import { expect } from '@open-wc/testing';
3-
import { removeDOsNotInSelection } from './utils.js';
3+
import { removeDOsNotInSelection, computeOrphanedRemoves } from './utils.js';
4+
import { nsdSpeced } from '../scl-template-update.testfiles.js';
45

56
describe('foundation/utils', () => {
67
describe('removeDOsNotInSelection', () => {
@@ -85,4 +86,83 @@ describe('foundation/utils', () => {
8586
expect(result.tagName).to.equal('LNodeType');
8687
});
8788
});
89+
90+
describe('computeOrphanedRemoves', () => {
91+
const mmxuId = 'MMXU$oscd$_c53e78191fabefa3';
92+
let dataTypeTemplates: Element;
93+
94+
beforeEach(() => {
95+
dataTypeTemplates = new DOMParser()
96+
.parseFromString(nsdSpeced, 'application/xml')
97+
.querySelector('DataTypeTemplates')!;
98+
});
99+
100+
function makeNewMmxu(doNames: string[]): Element {
101+
const doElements = doNames
102+
.map(name => {
103+
const existing = dataTypeTemplates.querySelector(
104+
`LNodeType[id="${mmxuId}"] > DO[name="${name}"]`
105+
);
106+
return existing ? existing.outerHTML : '';
107+
})
108+
.join('');
109+
return new DOMParser().parseFromString(
110+
`<LNodeType xmlns="http://www.iec.ch/61850/2003/SCL" lnClass="MMXU" id="${mmxuId}">${doElements}</LNodeType>`,
111+
'application/xml'
112+
).documentElement;
113+
}
114+
115+
it('returns the old LNodeType as the first remove', () => {
116+
const result = computeOrphanedRemoves(dataTypeTemplates, mmxuId, [
117+
makeNewMmxu(['Beh']),
118+
]);
119+
120+
expect(result).to.have.length.greaterThan(0);
121+
expect((result[0] as any).node as Element).to.equal(
122+
dataTypeTemplates.querySelector(`LNodeType[id="${mmxuId}"]`)
123+
);
124+
});
125+
126+
it('cascades orphans when A DO is removed: removes A DOType and all its exclusive descendants', () => {
127+
const result = computeOrphanedRemoves(dataTypeTemplates, mmxuId, [
128+
makeNewMmxu(['Beh']),
129+
]);
130+
131+
const orphanedIds = result
132+
.slice(1) // skip old LNodeType remove
133+
.map((edit: any) => edit.node.getAttribute('id'))
134+
.filter(id => id !== null);
135+
expect(orphanedIds).to.include('A$oscd$_41824603f63b26ac');
136+
expect(orphanedIds).to.include('phsA$oscd$_995aad120f12c815');
137+
expect(orphanedIds).to.include('cVal$oscd$_80272042468595d1');
138+
expect(orphanedIds).to.include('units$oscd$_3f2e10def85bfeac');
139+
expect(orphanedIds).to.include('mag$oscd$_ed49c2f7a55ad05a');
140+
expect(orphanedIds).to.include('SIUnit$oscd$_39f6ca400c633081');
141+
});
142+
143+
it('does not orphan the Beh DOType which is shared with LLN0', () => {
144+
const result = computeOrphanedRemoves(dataTypeTemplates, mmxuId, [
145+
makeNewMmxu(['Beh']),
146+
]);
147+
148+
const orphanedIds = result
149+
.slice(1) // skip old LNodeType remove
150+
.map((edit: any) => edit.node.getAttribute('id'))
151+
.filter(id => id !== null);
152+
153+
expect(orphanedIds).to.not.include('Beh$oscd$_c6ed035c8137b35a');
154+
expect(orphanedIds).to.not.include('stVal$oscd$_48ba16345b8e7f5b');
155+
});
156+
157+
it('returns only the old LNodeType when the new version keeps all DOs', () => {
158+
const result = computeOrphanedRemoves(dataTypeTemplates, mmxuId, [
159+
makeNewMmxu(['Beh', 'A']),
160+
]);
161+
162+
expect(result).to.have.lengthOf(1);
163+
expect((result[0] as any).node as Element).to.equal(
164+
dataTypeTemplates.querySelector(`LNodeType[id="${mmxuId}"]`)
165+
);
166+
});
167+
});
88168
});

‎foundation/utils.ts‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { Tree, TreeSelection } from '@openenergytools/tree-grid';
2+
import { EditV2 } from '@openscd/oscd-api';
23

34
export function getLNodeTypes(doc: XMLDocument | undefined): Element[] {
45
return Array.from(
@@ -69,3 +70,64 @@ export function removeDOsNotInSelection(
6970

7071
return clonedLNodeType;
7172
}
73+
74+
/**
75+
* Builds the Remove edits needed when an LNodeType is replaced.
76+
* Clones the DataTypeTemplates once, adds the incoming new nodes, removes
77+
* the old LNodeType, then cascades to find which pre-existing sub-types are
78+
* now unreferenced. Returns the old LNodeType remove plus all orphan removes.
79+
* @param dataTypeTemplates The live DataTypeTemplates element to base the remove calculations on
80+
* @param oldLNodeTypeId The ID of the LNodeType being replaced
81+
* @param newNodes The new nodes being added (cloned before passing in)
82+
* @returns An array of EditV2 objects representing the nodes to remove
83+
*/
84+
export function computeOrphanedRemoves(
85+
dataTypeTemplates: Element,
86+
oldLNodeTypeId: string,
87+
newNodes: Element[]
88+
): EditV2[] {
89+
const subTypes = Array.from(
90+
dataTypeTemplates.querySelectorAll(
91+
':scope > DOType, :scope > DAType, :scope > EnumType'
92+
)
93+
);
94+
const subTypeIds = subTypes
95+
.map(el => el.getAttribute('id'))
96+
.filter(id => id !== null);
97+
const preEditIds = new Set(subTypeIds);
98+
99+
const dttClone = dataTypeTemplates.cloneNode(true) as Element;
100+
newNodes.forEach(n => dttClone.appendChild(n.cloneNode(true)));
101+
dttClone
102+
.querySelector(`:scope > LNodeType[id="${oldLNodeTypeId}"]`)
103+
?.remove();
104+
105+
const remainingIds = new Set(preEditIds);
106+
const orphanIds: string[] = [];
107+
let changed = true;
108+
109+
while (changed) {
110+
changed = false;
111+
for (const id of remainingIds) {
112+
if (!dttClone.querySelector(`:scope *[type="${id}"]`)) {
113+
orphanIds.push(id);
114+
remainingIds.delete(id);
115+
dttClone.querySelector(`:scope > *[id="${id}"]`)?.remove();
116+
changed = true;
117+
}
118+
}
119+
}
120+
121+
const results: EditV2[] = [];
122+
const oldLN = dataTypeTemplates.querySelector(
123+
`:scope > LNodeType[id="${oldLNodeTypeId}"]`
124+
);
125+
if (oldLN) results.push({ node: oldLN as Node });
126+
orphanIds.forEach(id => {
127+
const node = dataTypeTemplates.querySelector(
128+
`:scope > *[id="${id}"]`
129+
) as Node | null;
130+
if (node) results.push({ node });
131+
});
132+
return results;
133+
}

‎scl-template-update.spec.ts‎

Lines changed: 29 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -372,19 +372,14 @@ describe('NsdTemplateUpdater', () => {
372372
const updateEdits = listener.args[0][0].detail.edit;
373373
expect(updateEdits).to.have.length.greaterThan(0);
374374

375-
// Verify the LNodeType was updated (not deleted)
376-
const lNodeType = element.doc?.querySelector(
377-
'LNodeType[id="MMXU$oscd$_c53e78191fabefa3"]'
375+
// Verify that the new LNodeType node should not contain 'A' but should contain 'Beh'
376+
const lNodeTypeInsert = updateEdits.find(
377+
(e: any) => 'node' in e && e.node.tagName === 'LNodeType'
378378
);
379-
expect(lNodeType).to.exist;
380-
381-
// Verify 'A' DO was removed
382-
const doA = lNodeType?.querySelector('DO[name="A"]');
383-
expect(doA).to.not.exist;
384-
385-
// Verify 'Beh' DO still exists
386-
const doBeh = lNodeType?.querySelector('DO[name="Beh"]');
387-
expect(doBeh).to.exist;
379+
expect(lNodeTypeInsert).to.exist;
380+
const newLNodeType = lNodeTypeInsert.node as Element;
381+
expect(newLNodeType.querySelector('DO[name="A"]')).to.not.exist;
382+
expect(newLNodeType.querySelector('DO[name="Beh"]')).to.exist;
388383
}).timeout(5000);
389384

390385
it('updates description when making selection changes', async () => {
@@ -412,11 +407,17 @@ describe('NsdTemplateUpdater', () => {
412407

413408
expect(listener).to.have.been.called;
414409

415-
// Verify the description was updated
416-
const lNodeType = element.doc?.querySelector(
417-
'LNodeType[id="MMXU$oscd$_c53e78191fabefa3"]'
410+
// Verify that description should appear either in the inserted LNodeType node
411+
// or as an attribute update, depending on whether the selection changed
412+
const edits = listener.args[0][0].detail.edit;
413+
const hasDescUpdate = edits.some(
414+
(e: any) =>
415+
('attributes' in e && e.attributes.desc === 'Updated with new DOs') ||
416+
('node' in e &&
417+
(e.node as Element).tagName === 'LNodeType' &&
418+
(e.node as Element).getAttribute('desc') === 'Updated with new DOs')
418419
);
419-
expect(lNodeType?.getAttribute('desc')).to.equal('Updated with new DOs');
420+
expect(hasDescUpdate).to.be.true;
420421
}).timeout(5000);
421422

422423
it('clears description when set to empty string', async () => {
@@ -440,6 +441,8 @@ describe('NsdTemplateUpdater', () => {
440441
).click();
441442
await element.updateComplete;
442443

444+
// Simulate the edit being applied to the document (normally done by an external editor)
445+
element.selectedLNodeType?.setAttribute('desc', 'Test Description');
443446
listener.resetHistory();
444447

445448
// Now clear the description
@@ -454,11 +457,17 @@ describe('NsdTemplateUpdater', () => {
454457

455458
expect(listener).to.have.been.called;
456459

457-
// Verify the description attribute was removed
458-
const lNodeType = element.doc?.querySelector(
459-
'LNodeType[id="MMXU$oscd$_c53e78191fabefa3"]'
460+
// Verify that description should be cleared, either as a null attribute update
461+
// or the inserted LNodeType node should lack the desc attribute
462+
const edits = listener.args[0][0].detail.edit;
463+
const hasDescClear = edits.some(
464+
(e: any) =>
465+
('attributes' in e && e.attributes.desc === null) ||
466+
('node' in e &&
467+
(e.node as Element).tagName === 'LNodeType' &&
468+
!(e.node as Element).hasAttribute('desc'))
460469
);
461-
expect(lNodeType?.hasAttribute('desc')).to.be.false;
470+
expect(hasDescClear).to.be.true;
462471
}).timeout(5000);
463472
});
464473

‎scl-template-update.ts‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ import {
4242
isLNodeTypeReferenced,
4343
filterSelection,
4444
removeDOsNotInSelection,
45+
computeOrphanedRemoves,
4546
} from './foundation/utils.js';
4647

4748
export default class NsdTemplateUpdated extends ScopedElementsMixin(
@@ -263,14 +264,18 @@ export default class NsdTemplateUpdated extends ScopedElementsMixin(
263264

264265
this.showSuccessFeedback(lnID, 'update');
265266
} else {
266-
// Swap mode: Insert new, then remove old with squash
267+
// Swap mode: Insert new, then remove old LNodeType and orphaned types with squash
268+
const dataTypeTemplates =
269+
this.selectedLNodeType!.closest('DataTypeTemplates')!;
270+
const remove = computeOrphanedRemoves(
271+
dataTypeTemplates,
272+
this.selectedLNodeType!.getAttribute('id')!,
273+
inserts.map(i => (i as { node: Node }).node as Element)
274+
);
275+
267276
this.dispatchEvent(newEditEventV2(inserts));
268277
await this.updateComplete;
269278

270-
const remove = removeDataType(
271-
{ node: this.selectedLNodeType! },
272-
{ force: true }
273-
);
274279
this.dispatchEvent(
275280
newEditEventV2(remove, { squash: true, title: `Update ${lnID}` })
276281
);
@@ -324,10 +329,12 @@ export default class NsdTemplateUpdated extends ScopedElementsMixin(
324329
const supportingTypes = inserts.filter(
325330
insert => insert !== lNodeTypeInsert
326331
);
327-
const removeOld = removeDataType(
328-
{ node: currentLNodeType },
329-
{ force: true }
330-
);
332+
333+
const dataTypeTemplates = currentLNodeType.closest('DataTypeTemplates')!;
334+
const removeOld = computeOrphanedRemoves(dataTypeTemplates, lnID, [
335+
newLNodeType,
336+
...(supportingTypes as { node: Element }[]).map(i => i.node),
337+
]);
331338

332339
return [
333340
...supportingTypes,

0 commit comments

Comments
 (0)