fix: removeIED - prevent duplicate LNode's

.gitignore

@@ -2,3 +2,8 @@ node_modules
 coverage
 dist
 doc
+
+.vscode
+.idea
+*.log
+*.tsbuildinfo

index.ts

@@ -11,6 +11,7 @@ export { updateLnType } from "./tSubstation/updateLnType.js";
 export { InsertIedOptions, insertIed } from "./tIED/insertIED.js";
 export { updateIED } from "./tIED/updateIED.js";
 export { removeIED } from "./tIED/removeIED.js";
+export type { RemoveIedOptions } from "./tIED/removeIED.js";
 
 export { findControlBlockSubscription } from "./tControl/findControlSubscription.js";
 export { controlBlockObjRef } from "./tControl/controlBlockObjRef.js";

tIED/removeIED.spec.ts

@@ -5,7 +5,7 @@ import { isRemove, isUpdate } from "@openscd/oscd-api/utils.js";
 
 import { handleEdit } from "../foundation/helpers.test.js";
 
-import { scl } from "./removeIED.testfile.js";
+import { scl, sclDuplicateLNodes } from "./removeIED.testfile.js";
 
 import { removeIED } from "./removeIED.js";
 
@@ -47,10 +47,10 @@ describe("Function to an remove the IED and its referenced elements", () => {
     expect(removeIED({ node: publi }).length).to.equal(0);
   });
 
-  it("updates LNode iedName attributes to None as well", () => {
+  it("removes all bound LNodes", () => {
     const edits = removeIED({ node: subscriber1 });
 
-    expect(numberUpdates(edits, "LNode")).to.equal(1);
+    expect(numberRemoves(edits, "LNode")).to.equal(1);
   });
 
   it("removes ConnectedAPs as well", () => {
@@ -126,4 +126,136 @@ describe("Function to an remove the IED and its referenced elements", () => {
     // 1 supervised control block is not subscribed so is not removed
     expect(after.length).to.equal(1);
   });
+
+  describe("referenced LNode's", () => {
+    /*
+     * 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.
+     */
+    describe("without 'preservveNodes' set (default)", () => {
+      //TODO consider changing this into a forEach (Substation, VL and Bay) array test.
+      ["Bay", "VoltageLevel", "Substation"].forEach((scope) => {
+        it(`deletes all LNodes found directly within a ${scope}`, () => {
+          const sclDom = new DOMParser().parseFromString(
+            sclDuplicateLNodes,
+            "application/xml",
+          );
+          const iedA = sclDom.querySelector('IED[name="IED_A"]')!;
+          const beforeSpec_LNodeCount = (
+            sclDom.querySelectorAll(`${scope} > LNode[iedName='None']`) ?? []
+          ).length;
+
+          const edits = removeIED({ node: iedA });
+          handleEdit(edits);
+          const after_iedA_lNodes = Array.from(
+            sclDom.querySelectorAll(`${scope} LNode[iedName="IED_A"]`),
+          ).length;
+          const after_spec_LNodeCount = (
+            sclDom.querySelectorAll(`${scope} > LNode[iedName='None']`) ?? []
+          ).length;
+          expect(after_iedA_lNodes).to.equal(0);
+          // The number of LNodes set to None should not have changed.
+          expect(after_spec_LNodeCount).to.equal(beforeSpec_LNodeCount);
+
+          //
+        });
+      });
+    });
+
+    describe.only("with preserveLNodes set", () => {
+      // Broke this into 3 separate tests, so the scope of the failure "might" be narrower.
+      // Do keep in mind however, the subject SCL has 2 of everything. E.g. S1 & S2
+      ["Bay", "VoltageLevel", "Substation"].forEach((scope) => {
+        it(`Within a ${scope}, it sets all bound LNodes to None`, () => {
+          //we're using the "duplicates" test file, but by only deleting 1 IED, no duplicates occur (yet).
+          const sclDom = new DOMParser().parseFromString(
+            sclDuplicateLNodes,
+            "application/xml",
+          );
+          const iedA = sclDom.querySelector('IED[name="IED_A"]')!;
+          const beforeSpec_LNodeCount = (
+            sclDom.querySelectorAll(`${scope} > LNode[iedName='None']`) ?? []
+          ).length;
+          const beforeIedA_LNodeCount = (
+            sclDom.querySelectorAll(`${scope} > LNode[iedName='IED_A']`) ?? []
+          ).length;
+
+          const edits = removeIED({ node: iedA }, { preserveLNodes: true });
+          handleEdit(edits);
+          const lNodes = Array.from(
+            sclDom.querySelectorAll(`${scope} > LNode[iedName="None"]`),
+          );
+          expect(lNodes.length).to.equal(
+            beforeSpec_LNodeCount + beforeIedA_LNodeCount,
+          );
+          //
+        });
+      });
+
+      ["Bay", "VoltageLevel", "Substation"].forEach((scope) => {
+        it(`Within a ${scope}, it removes 'would-be' duplicates`, () => {
+          //we're using the "duplicates" test file, but by only deleting 1 IED, no duplicates occur.
+          const sclDom = new DOMParser().parseFromString(
+            sclDuplicateLNodes,
+            "application/xml",
+          );
+          const iedA = sclDom.querySelector('IED[name="IED_A"]')!;
+          const iedB = sclDom.querySelector('IED[name="IED_B"]')!;
+
+          handleEdit(removeIED({ node: iedA }, { preserveLNodes: true }));
+          const beforeSpec_LNodeCount = (
+            sclDom.querySelectorAll(`${scope} > LNode[iedName='None']`) ?? []
+          ).length;
+          // After the first wave of deletions the SCL already has LNodes(iedName=None),
+          // which exactly match the LNodes we're about to remove.
+          // So when IED_B is removed (with preserveLNodes set), the LNodes(None)
+          // should not have changed.
+          handleEdit(removeIED({ node: iedB }, { preserveLNodes: true }));
+          const iedB_lNodesCount = Array.from(
+            sclDom.querySelectorAll(`${scope} > LNode[iedName="IED_B"]`),
+          ).length;
+          expect(iedB_lNodesCount).to.equal(0);
+          // Although we've removed IED_A and IED_B, the count should remain unchanged after
+          // removing IED_A, because both IED's are bound exactly the same.
+          const afterSpec_LNodeCount = (
+            sclDom.querySelectorAll(`${scope} > LNode[iedName='None']`) ?? []
+          ).length;
+          expect(afterSpec_LNodeCount).to.equal(beforeSpec_LNodeCount);
+        });
+      });
+
+      it("does not create duplicate LNode keys when removing both IEDs", () => {
+        const sclDom = new DOMParser().parseFromString(
+          sclDuplicateLNodes,
+          "application/xml",
+        );
+        const iedA = sclDom.querySelector('IED[name="IED_A"]')!;
+        const iedB = sclDom.querySelector('IED[name="IED_B"]')!;
+
+        handleEdit(removeIED({ node: iedA }));
+        handleEdit(removeIED({ node: iedB }));
+
+        const ce = sclDom.querySelector('ConductingEquipment[name="QA1"]')!;
+        const lNodes = Array.from(ce.querySelectorAll(":scope > LNode"));
+        const keys = lNodes.map(
+          (ln) =>
+            `${ln.getAttribute("ldInst")}|${ln.getAttribute(
+              "lnClass",
+            )}|${ln.getAttribute("lnInst")}|${ln.getAttribute(
+              "prefix",
+            )}|${ln.getAttribute("iedName")}`,
+        );
+        const uniqueKeys = new Set(keys);
+
+        expect(keys.length).to.equal(
+          uniqueKeys.size,
+          `Duplicate LNode keys found: ${keys
+            .filter((k, i) => keys.indexOf(k) !== i)
+            .join(", ")}`,
+        );
+      });
+    });
+  });
 });

tIED/removeIED.testfile.ts

@@ -575,3 +575,65 @@ export const scl = `<SCL xmlns="http://www.iec.ch/61850/2003/SCL" xmlns:esld="ht
   </EnumType>
 </DataTypeTemplates>
 </SCL>`;
+
+/** 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. */
+export const sclDuplicateLNodes = `<SCL xmlns="http://www.iec.ch/61850/2003/SCL" version="2007" revision="B" release="4">
+<Header id="DuplicateLNodes"/>
+<Substation name="S1">
+  <LNode iedName="IED_A" ldInst="CBSW" lnClass="XCBR" lnInst="1" prefix=""/>
+  <LNode iedName="IED_B" ldInst="CBSW" lnClass="XCBR" lnInst="1" prefix=""/>
+  <VoltageLevel name="V1">
+    <LNode iedName="IED_A" ldInst="CBSW" lnClass="XCBR" lnInst="1" prefix=""/>
+    <LNode iedName="IED_B" ldInst="CBSW" lnClass="XCBR" lnInst="1" prefix=""/>
+    <Bay name="B1">
+      <LNode iedName="IED_A" ldInst="CBSW" lnClass="XCBR" lnInst="1" prefix=""/>
+      <LNode iedName="IED_B" ldInst="CBSW" lnClass="XCBR" lnInst="1" prefix=""/>
+      <ConductingEquipment name="QA1" type="CBR">
+        <LNode iedName="IED_A" ldInst="CBSW" lnClass="XCBR" lnInst="1" prefix=""/>
+        <LNode iedName="IED_B" ldInst="CBSW" lnClass="XCBR" lnInst="1" prefix=""/>
+        <LNode iedName="IED_A" ldInst="CBSW" lnClass="CSWI" lnInst="1" prefix=""/>
+        <LNode iedName="IED_B" ldInst="CBSW" lnClass="CSWI" lnInst="1" prefix=""/>
+      </ConductingEquipment>
+    </Bay>
+  </VoltageLevel>
+</Substation>
+
+<Substation name="S2">
+  <VoltageLevel name="V2">
+    <Bay name="B2">
+      <ConductingEquipment name="QA1" type="CBR">
+        <LNode iedName="IED_A" ldInst="CBSW" lnClass="XCBR" lnInst="1" prefix=""/>
+        <LNode iedName="IED_B" ldInst="CBSW" lnClass="XCBR" lnInst="1" prefix=""/>
+        <LNode iedName="IED_A" ldInst="CBSW" lnClass="CSWI" lnInst="1" prefix=""/>
+        <LNode iedName="IED_B" ldInst="CBSW" lnClass="CSWI" lnInst="1" prefix=""/>
+      </ConductingEquipment>
+    </Bay>
+  </VoltageLevel>
+</Substation>
+<IED name="IED_A" manufacturer="Dummy">
+  <AccessPoint name="AP1">
+    <Server>
+      <Authentication/>
+      <LDevice inst="CBSW">
+        <LN0 lnClass="LLN0" inst="" lnType="Dummy.LLN0"/>
+        <LN lnClass="XCBR" inst="1" lnType="Dummy.XCBR"/>
+        <LN lnClass="CSWI" inst="1" lnType="Dummy.CSWI"/>
+      </LDevice>
+    </Server>
+  </AccessPoint>
+</IED>
+<IED name="IED_B" manufacturer="Dummy">
+  <AccessPoint name="AP1">
+    <Server>
+      <Authentication/>
+      <LDevice inst="CBSW">
+        <LN0 lnClass="LLN0" inst="" lnType="Dummy.LLN0"/>
+        <LN lnClass="XCBR" inst="1" lnType="Dummy.XCBR"/>
+        <LN lnClass="CSWI" inst="1" lnType="Dummy.CSWI"/>
+      </LDevice>
+    </Server>
+  </AccessPoint>
+</IED>
+</SCL>`;

tIED/removeIED.ts

@@ -6,8 +6,6 @@ import { removeSubscriptionSupervision } from "../tLN/removeSubscriptionSupervis
 
 const elementsToRemove = ["Association", "ClientLN", "ConnectedAP", "KDC"];
 
-const elementsToReplaceWithNone = ["LNode"];
-
 function removeIEDNameTextContent(ied: Element, iedName: string): Remove[] {
   return Array.from(ied.ownerDocument.getElementsByTagName("IEDName"))
     .filter(isPublic)
@@ -50,16 +48,94 @@ function removeIedSubscriptionsAndSupervisions(
   return [...extRefRemovals, ...supervisionRemovals];
 }
 
-function updateIedNameToNone(ied: Element, iedName: string): SetAttributes[] {
-  const selector = elementsToReplaceWithNone
-    .map((iedNameElement) => `${iedNameElement}[iedName="${iedName}"]`)
-    .join(",");
+const lNodeKey = (ln: Element): string =>
+  ["lnClass", "lnInst", "ldInst", "prefix"].map((a) => ln.getAttribute(a) ?? "").join("|");
 
-  return Array.from(ied.ownerDocument.querySelectorAll(selector))
-    .filter(isPublic)
-    .map((element) => {
-      return { element, attributes: { iedName: "None" } };
-    });
+const getLNodeScopeElement = (ln: Element): Element | null => {
+  if (ln.closest("Private") === null) {
+    return ln.closest("Bay, VoltageLevel, Substation");
+  }
+  return null;
+};
+
+const removeLNode = (ln: Element): Remove => {
+  return { node: ln };
+};
+
+const setLNodeToNone = (ln: Element): SetAttributes => {
+  return { element: ln, attributes: { iedName: "None" } };
+};
+
+const getLNodesByIedName = (doc: XMLDocument, name: string): Element[] => {
+  return Array.from(
+    doc.querySelectorAll(`Substation LNode[iedName=${name}]`) ?? [],
+  ).filter(isPublic);
+};
+
+/**
+ * Default handling for LNodes - find any (public) matching LNodes and create a Remove edit for them.
+ */
+function removeBoundLNodes(ied: Element, name: string): Remove[] {
+  return (getLNodesByIedName(ied.ownerDocument, name) ?? []).map(removeLNode);
+}
+
+/**
+ * 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
+ * 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.
+ */
+function detachLNodeBindings(
+  ied: Element,
+  name: string,
+): (SetAttributes | Remove)[] {
+  const doc = ied.ownerDocument;
+  const boundNodes = getLNodesByIedName(doc, name);
+
+  if (boundNodes.length === 0) {
+    return [];
+  }
+
+  const UnboundLNodesByScope = new Map<Element, Set<string>>();
+  getLNodesByIedName(doc, "None").forEach((ln) => {
+    const scope = getLNodeScopeElement(ln);
+    if (scope !== null) {
+      let keys = UnboundLNodesByScope.get(scope);
+      if (!keys) {
+        keys = new Set<string>();
+        UnboundLNodesByScope.set(scope, keys);
+      }
+      keys.add(lNodeKey(ln));
+    }
+  });
+
+  return boundNodes
+    .map((ln) => {
+      const scope = getLNodeScopeElement(ln);
+      if (!scope) {
+        return;
+      }
+
+      const keys = UnboundLNodesByScope.get(scope);
+
+      const key = lNodeKey(ln);
+      if (keys && keys.has(key)) {
+        return removeLNode(ln);
+      } else {
+        return setLNodeToNone(ln);
+      }
+    })
+    .filter((edit): edit is SetAttributes | Remove => edit !== undefined);
+}
+
+/** Options for the {@link removeIED} function. */
+export interface RemoveIedOptions {
+  /** Flag to optionally set all bound LNodes to iedName="None". Defaults to `false`.
+   * Note: If setting an LNode to "None" would result in two matching LNodes, the
+   * LNode will be simply deleted instead.*/
+  preserveLNodes?: boolean;
 }
 
 /**
@@ -69,12 +145,19 @@ function updateIedNameToNone(ied: Element, iedName: string): SetAttributes[] {
  * 1. Remove all elements which should no longer exist including ClientLN,
  *    KDC, Association, ConnectedAP and IEDName
  * 2. Remove subscriptions and supervisions
- * 2. Update LNodes to an iedName of None
+ * 3. By default removes all LNodes bound to this IED.
+ * 4. By setting the optional "preserveLNodes" option to true,
+ *    bound LNodes are set to iedName="None" and only removed if
+ *    it would result in two matching LNodes.
  * ```
  * @param remove - IED element as a Remove edit
+ * @param options - Optional settings to control removal behavior
  * @returns - Set of additional edits to relevant SCL elements
  */
-export function removeIED(remove: Remove): (SetAttributes | Remove)[] {
+export function removeIED(
+  remove: Remove,
+  options: RemoveIedOptions = { preserveLNodes: false },
+): (SetAttributes | Remove)[] {
   if (
     remove.node.nodeType !== Node.ELEMENT_NODE ||
     remove.node.nodeName !== "IED" ||
@@ -90,6 +173,8 @@ export function removeIED(remove: Remove): (SetAttributes | Remove)[] {
     ...removeIEDNameTextContent(ied, name),
     ...removeWithIedName(ied, name),
     ...removeIedSubscriptionsAndSupervisions(ied, name),
-    ...updateIedNameToNone(ied, name),
+    ...(options.preserveLNodes
+      ? detachLNodeBindings(ied, name)
+      : removeBoundLNodes(ied, name)),
   ];
 }