From 70531b9968fc79c2dcccced67eeb0c4c91a309de Mon Sep 17 00:00:00 2001 From: Daniel Mulholland Date: Tue, 20 Oct 2026 07:36:23 +1000 Subject: [PATCH] fix(tIED): scope LNode deduplication to immediate parent container The SCL schema applies the LNode uniqueness constraint to the LNode's immediate parent container, but detachLNodeBindings grouped unbound LNodes by the nearest Bay, VoltageLevel or Substation. A bound LNode in one ConductingEquipment could therefore be deleted because an unbound LNode with the same key existed in a sibling container under the same Bay. Use the parent element as the scope and add each LNode changed to iedName="None" to the cache, so duplicate keys among the bound LNodes handled in a single call are also detected. Closes #158 --- tIED/removeIED.spec.ts | 75 ++++++++++++++++++++++++++++++++++++-- tIED/removeIED.testfile.ts | 37 +++++++++++++++++++ tIED/removeIED.ts | 46 ++++++++++++++--------- 3 files changed, 137 insertions(+), 21 deletions(-) diff --git a/tIED/removeIED.spec.ts b/tIED/removeIED.spec.ts index e65eac0..2959042 100644 --- a/tIED/removeIED.spec.ts +++ b/tIED/removeIED.spec.ts @@ -5,7 +5,12 @@ import { isRemove, isUpdate } from "@openscd/oscd-api/utils.js"; import { handleEdit } from "../foundation/helpers.test.js"; -import { scl, sclDuplicateLNodes } from "./removeIED.testfile.js"; +import { + scl, + sclDuplicateBoundLNodes, + sclDuplicateLNodes, + sclLNodesInSeparateContainers, +} from "./removeIED.testfile.js"; import { removeIED } from "./removeIED.js"; @@ -137,8 +142,8 @@ describe("Function to an remove the IED and its referenced elements", () => { /* * Here we need to test: * - Delete all LNode references found with matching iedName, BUT only inside the substation section (but not inside Private sections). - * - Find all LNode references with matching iedName and either set them to None, or delete them if setting them to None would result in duplicate LNode keys within the same scope. - * The scope is defined as the nearest Bay, VL or Substation parent. + * - Find all LNode references with matching iedName and either set them to None, or delete them if setting them to None would result in duplicate LNode keys within the same container. + * The scope is defined as the LNode's immediate parent, matching the schema's uniqueness constraint. */ describe("without 'preserveNodes' set (default)", () => { ["Bay", "VoltageLevel", "Substation"].forEach((scope) => { @@ -234,6 +239,70 @@ describe("Function to an remove the IED and its referenced elements", () => { }); }); + it("does not remove a bound LNode because an unbound copy exists in another container", () => { + const sclDom = new DOMParser().parseFromString( + sclLNodesInSeparateContainers, + "application/xml", + ); + const ied = sclDom.querySelector('IED[name="IED1"]')!; + const bound = sclDom.querySelector( + 'ConductingEquipment[name="QB1"] LNode', + )!; + + const edits = removeIED({ node: ied }, { preserveLNodes: true }); + + expect(edits.some((edit) => isRemove(edit) && edit.node === bound)).to + .be.false; + expect( + edits.some( + (edit) => + isUpdate(edit) && + edit.element === bound && + edit.attributes?.iedName === "None", + ), + ).to.be.true; + + handleEdit(edits); + + expect( + sclDom.querySelector( + 'ConductingEquipment[name="QB1"] LNode[iedName="None"]', + ), + ).to.not.be.null; + expect( + sclDom.querySelector( + 'ConductingEquipment[name="QA1"] LNode[iedName="None"]', + ), + ).to.not.be.null; + }); + + it("sets only one of two matching bound LNodes in the same container to None", () => { + const sclDom = new DOMParser().parseFromString( + sclDuplicateBoundLNodes, + "application/xml", + ); + const ied = sclDom.querySelector('IED[name="IED1"]')!; + + const edits = removeIED({ node: ied }, { preserveLNodes: true }); + + const lNodeEdits = edits.filter( + (edit) => + (isRemove(edit) && edit.node.nodeName === "LNode") || + (isUpdate(edit) && edit.element.nodeName === "LNode"), + ); + expect(lNodeEdits.length).to.equal(2); + expect(lNodeEdits.filter(isRemove).length).to.equal(1); + expect(lNodeEdits.filter(isUpdate).length).to.equal(1); + + handleEdit(edits); + + const lNodes = sclDom.querySelectorAll( + 'ConductingEquipment[name="QA1"] > LNode', + ); + expect(lNodes.length).to.equal(1); + expect(lNodes[0].getAttribute("iedName")).to.equal("None"); + }); + it("does not create duplicate LNode keys when removing both IEDs", () => { const sclDom = new DOMParser().parseFromString( sclDuplicateLNodes, diff --git a/tIED/removeIED.testfile.ts b/tIED/removeIED.testfile.ts index 81b403c..0e1eae1 100644 --- a/tIED/removeIED.testfile.ts +++ b/tIED/removeIED.testfile.ts @@ -576,6 +576,43 @@ export const scl = ` + + + + + + + + + + + + + + + +`; + +/** SCL with two bound LNodes sharing the same key in one container. */ +export const sclDuplicateBoundLNodes = ` + + + + + + + + + + + + + +`; + /** SCL with two IEDs that have LNodes in the same ConductingEquipment * with matching (ldInst, lnClass, lnInst, prefix). Removing both IEDs * sequentially should not create duplicate LNode key sequences. */ diff --git a/tIED/removeIED.ts b/tIED/removeIED.ts index cf6c1d9..575b4ac 100644 --- a/tIED/removeIED.ts +++ b/tIED/removeIED.ts @@ -53,9 +53,12 @@ const lNodeKey = (ln: Element): string => .map((a) => ln.getAttribute(a) ?? "") .join("|"); -const getLNodeScopeElement = (ln: Element): Element => { - return ln.closest("Bay, VoltageLevel, Substation")!; -}; +/** + * The SCL schema applies the LNode uniqueness constraint to the immediate + * parent container of the LNode (e.g. a ConductingEquipment or a Function), + * so that is also the scope in which duplicate keys have to be detected. + */ +const getLNodeScopeElement = (ln: Element): Element => ln.parentElement!; const createRemoveEdit = (ln: Element): Remove => { return { node: ln }; @@ -82,9 +85,11 @@ function removeBoundLNodes(ied: Element, name: string): Remove[] { * Build the edits required to detach all public LNode bindings to `iedName` * from the substation model, preserving each as a specification entry with * (iedName="None"). A cache of all LNodes (with iedName="None") is first built - * up (grouped by their scope/container). This is used to check if changing the + * up (grouped by their parent container). This is used to check if changing the * iedName of a bound LNode to "None" would create a duplicate binding within its - * scope. If this would result in a duplicate, the LNode is simply removed instead. + * container. If this would result in a duplicate, the LNode is simply removed + * instead. The cache is updated while processing, so duplicates within the set + * of bound LNodes removed in this call are handled as well. */ function detachLNodeBindings( ied: Element, @@ -108,19 +113,24 @@ function detachLNodeBindings( keys.add(lNodeKey(ln)); }); - return boundNodes - .map((ln) => { - const scope = getLNodeScopeElement(ln); - const keys = unboundLNodesByScope.get(scope); - - const key = lNodeKey(ln); - if (keys && keys.has(key)) { - return createRemoveEdit(ln); - } else { - return setLNodeToNone(ln); - } - }) - .filter((edit): edit is SetAttributes | Remove => edit !== undefined); + return boundNodes.map((ln) => { + const scope = getLNodeScopeElement(ln); + let keys = unboundLNodesByScope.get(scope); + if (!keys) { + keys = new Set(); + unboundLNodesByScope.set(scope, keys); + } + + const key = lNodeKey(ln); + if (keys.has(key)) { + return createRemoveEdit(ln); + } + + // Remember the key, so that a second bound LNode with the same key in the + // same container would not also be set to "None". + keys.add(key); + return setLNodeToNone(ln); + }); } /** Options for the {@link removeIED} function. */