Use toString() for online Set instead of interning Address

Address interning is a potential memory leak if a service creates
many distinct addresses. Instead, use addr.toString() for the
Set<string> operations on this.online in the directory.

Author: AlexGodbehere

acs-directory/lib/mqttcli.js

@@ -452,7 +452,7 @@ export default class MQTTCli {
 
     async on_birth(address, payload) {
         this.log("device", `Registering BIRTH for ${address}`);
-        this.online.add(address);
+        this.online.add(address.toString());
 
         let tree;
         if (payload.uuid === UUIDs.FactoryPlus) {
@@ -485,7 +485,7 @@ export default class MQTTCli {
         this.log("device", `Registering DEATH for ${address}`);
 
         this.alerts.delete(address);
-        this.online.delete(address);
+        this.online.delete(address.toString());
         await this.model.death({address, time});
 
         this.log("device", `Finished DEATH for ${address}`);
@@ -552,7 +552,7 @@ export default class MQTTCli {
          * rebirth any device we haven't seen a birth for, even if the
          * database says it's online. This is important because we might
          * have the wrong schema information. */
-        if (this.online.has(addr) || pending[addr]) return false;
+        if (this.online.has(addr.toString()) || pending[addr]) return false;
 
         /* Mark that we're working on this device and wait 5-10s to see if
          * it rebirths on its own. */
@@ -561,7 +561,7 @@ export default class MQTTCli {
 
         /* Clear our marker first so we retry next time */
         delete (pending[addr]);
-        if (this.online.has(addr)) return false;
+        if (this.online.has(addr.toString())) return false;
 
         sent[addr] = Date.now();
         return true;

lib/js-service-client/lib/sparkplug/util.js

@@ -59,8 +59,6 @@ export function sparkplug_safe_string (e) {
 
 /** Class representing a Sparkplug address. */
 export class Address {
-    static #cache = new Map();
-
     /** Construct an address.
      * Omitting the `device` or passing the empty string will create a
      * Node address.
@@ -71,19 +69,9 @@ export class Address {
     constructor (group, node, device) {
         if (device == null || device == "")
             device = undefined;
-
-        const key = device != null
-            ? `${group}/${node}/${device}`
-            : `${group}/${node}`;
-
-        const existing = Address.#cache.get(key);
-        if (existing) return existing;
-
         this.group = group;
         this.node = node;
         this.device = device;
-
-        Address.#cache.set(key, this);
     }
 
     /** Convert to Address.

lib/js-service-client/test/address.test.js

@@ -1,58 +1,50 @@
 import { describe, it, expect } from "vitest";
 import { Address } from "../lib/sparkplug/util.js";
 
-describe("Address interning", () => {
-    it("returns the same object for identical node addresses", () => {
-        const a = new Address("G", "N");
-        const b = new Address("G", "N");
-        expect(a).toBe(b);
-    });
-
-    it("returns the same object for identical device addresses", () => {
-        const a = new Address("G", "N", "D");
-        const b = new Address("G", "N", "D");
-        expect(a).toBe(b);
-    });
-
-    it("returns different objects for different addresses", () => {
-        const a = new Address("G", "N", "D1");
-        const b = new Address("G", "N", "D2");
-        expect(a).not.toBe(b);
-    });
-
+describe("Address", () => {
     it("treats empty string device as node address", () => {
         const a = new Address("G", "N", "");
-        const b = new Address("G", "N");
-        expect(a).toBe(b);
+        expect(a.device).toBeUndefined();
     });
 
     it("treats null device as node address", () => {
         const a = new Address("G", "N", null);
-        const b = new Address("G", "N");
-        expect(a).toBe(b);
+        expect(a.device).toBeUndefined();
     });
 
-    it("works correctly with Set", () => {
-        const set = new Set();
+    it("toString formats node address", () => {
+        const a = new Address("G", "N");
+        expect(a.toString()).toBe("G/N");
+    });
+
+    it("toString formats device address", () => {
         const a = new Address("G", "N", "D");
-        set.add(a);
+        expect(a.toString()).toBe("G/N/D");
+    });
 
+    it("equals compares by value", () => {
+        const a = new Address("G", "N", "D");
         const b = new Address("G", "N", "D");
-        expect(set.has(b)).toBe(true);
+        expect(a.equals(b)).toBe(true);
+    });
 
-        set.delete(b);
-        expect(set.has(a)).toBe(false);
+    it("equals returns false for different addresses", () => {
+        const a = new Address("G", "N", "D1");
+        const b = new Address("G", "N", "D2");
+        expect(a.equals(b)).toBe(false);
     });
 
-    it("parse returns interned instances", () => {
+    it("parse round-trips with toString", () => {
         const a = new Address("G", "N", "D");
-        const b = Address.parse("G/N/D");
-        expect(a).toBe(b);
+        const b = Address.parse(a.toString());
+        expect(b.equals(a)).toBe(true);
     });
 
-    it("parent_node returns interned instances", () => {
-        const a = new Address("G", "N");
-        const b = new Address("G", "N", "D").parent_node();
-        expect(a).toBe(b);
+    it("parent_node returns node address", () => {
+        const a = new Address("G", "N", "D");
+        const parent = a.parent_node();
+        expect(parent.group).toBe("G");
+        expect(parent.node).toBe("N");
+        expect(parent.device).toBeUndefined();
     });
 });