Ensure Records are treated as separate namespaces for IDs

src/ThingSetRegistry.cpp

@@ -17,6 +17,18 @@ static uint32_t calculateId(const uint16_t &id, const uint16_t &parentId)
     return offset + id + 1; // ensure never in same bucket as original
 }
 
+// Effective identity of a node within the registry: raw id for normal nodes,
+// parent-scoped id for record members. Two nodes are duplicates iff their
+// effective ids match
+static uint32_t effectiveId(ThingSetNode *node)
+{
+    uint32_t id = node->getId();
+    if (node->tryCastTo(ThingSetNodeType::recordMember, nullptr)) {
+        id = calculateId(id, node->getParentId());
+    }
+    return id;
+}
+
 ThingSetRegistry::ThingSetRegistry()
 {
     // manually register the root node
@@ -39,10 +51,7 @@ void ThingSetRegistry::registerOrUnregisterNode(
     ThingSetNode *node, std::function<void(NodeList &, ThingSetNode *)> nodeListAction,
     std::function<bool(ThingSetParentNode *, ThingSetNode *)> parentNodeAction)
 {
-    unsigned id = node->getId();
-    if (node->tryCastTo(ThingSetNodeType::recordMember, nullptr)) {
-        id = calculateId(id, node->getParentId());
-    }
+    uint32_t id = effectiveId(node);
     NodeList &list = _nodeMap[id % NODE_MAP_LOOKUP_BUCKETS];
     nodeListAction(list, node);
     ThingSetParentNode *parent;
@@ -56,10 +65,12 @@ void ThingSetRegistry::registerNode(ThingSetNode *node)
     LOG_DEBUG("Registering node %s (0x%x)", node->getName().data(), node->getId());
     ThingSetRegistry::instance().registerOrUnregisterNode(
         node, [](auto &l, auto *n) {
+            uint32_t nEff = effectiveId(n);
             for (const auto &en : l) {
-                if (en->getId() == n->getId()) {
-                    LOG_ERROR("Cannot register node %s (0x%x) as it already exists (under name %s)", n->getName().data(), n->getId(), en->getName().data());
-                    assert(en->getId() != n->getId());
+                if (effectiveId(en) == nEff) {
+                    LOG_ERROR("Cannot register node %s (0x%x under parent 0x%x) as it already exists (under name %s)",
+                              n->getName().data(), n->getId(), n->getParentId(), en->getName().data());
+                    assert(effectiveId(en) != nEff);
                     return;
                 }
             }
@@ -123,8 +134,16 @@ bool ThingSetRegistry::findById(const unsigned id, const unsigned parentId, Thin
     if (findParentById(parentId, &parent) && parent->tryCastTo(ThingSetNodeType::record, nullptr))
     {
         uint32_t fakeId = calculateId(id, parentId);
-        NodeList list = instance()._nodeMap[fakeId % NODE_MAP_LOOKUP_BUCKETS];
-        return findByIdInNodeList(list, id, node);
+        NodeList &list = instance()._nodeMap[fakeId % NODE_MAP_LOOKUP_BUCKETS];
+        // Multiple record-member proxies can share a bucket when they share a
+        // raw id under different parents. Filter by parent id too so the
+        // caller gets the proxy they asked for
+        for (ThingSetNode *n : list) {
+            if (n && n->getId() == id && n->getParentId() == parentId) {
+                *node = n;
+                return true;
+            }
+        }
     }
     return false;
 }

tests/CMakeLists.txt

@@ -27,6 +27,7 @@ target_sources(testapp PRIVATE TestBinaryEncoder.cpp
     TestIntrusiveLinkedList.cpp
     TestProperties.cpp
     TestRecordMembers.cpp
+    TestRecordMemberNamespaces.cpp
     TestFunctions.cpp
     TestSubsets.cpp
     TestAsioIpClientServer.cpp

tests/TestRecordMemberNamespaces.cpp

@@ -0,0 +1,73 @@
+/*
+ * Copyright (c) 2026 Brill Power.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+#include "gtest/gtest.h"
+#include <thingset++/ThingSet.hpp>
+
+// >>> NOTE <<<
+// 
+// If this test binary links successfully, static init didn't trip the
+// spurious dedup assert which is a regression in itself
+
+using namespace ThingSet;
+
+namespace {
+
+// Two distinct record types declaring members with the same raw template
+// ids (0x7BA, 0x7BB) under different parent record ids (0x7B0, 0x7B1).
+// Record members are namespaced by their parent record
+struct RecordA
+{
+    ThingSetReadOnlyRecordMember<0x7BA, 0x7B0, "alpha", uint64_t> alpha;
+    ThingSetReadOnlyRecordMember<0x7BB, 0x7B0, "beta", uint32_t> beta;
+};
+
+struct RecordB
+{
+    ThingSetReadOnlyRecordMember<0x7BA, 0x7B1, "alpha", uint64_t> alpha;
+    ThingSetReadOnlyRecordMember<0x7BB, 0x7B1, "beta", uint32_t> beta;
+};
+
+ThingSetReadOnlyProperty<std::array<RecordA, 2>> recordsA{ 0x7B0, 0x0, "recordsA" };
+ThingSetReadOnlyProperty<std::array<RecordB, 1>> recordsB{ 0x7B1, 0x0, "recordsB" };
+
+} // namespace
+
+TEST(RecordMemberNamespaces, BothParentRecordsRegister)
+{
+    ThingSetNode *node;
+    ASSERT_TRUE(ThingSetRegistry::findById(0x7B0, &node));
+    EXPECT_EQ("recordsA", node->getName());
+    ASSERT_TRUE(ThingSetRegistry::findById(0x7B1, &node));
+    EXPECT_EQ("recordsB", node->getName());
+}
+
+TEST(RecordMemberNamespaces, FindByIdDisambiguatesByParent)
+{
+    // Both record types share raw member id 0x7BA. Their proxies land in
+    // the same bucket because the parent id contribution to the bucket
+    // hash is a multiple of 8 and cancels. findById(rawId, parentId) must
+    // therefore also filter on parent id
+    ThingSetNode *alphaA = nullptr;
+    ASSERT_TRUE(ThingSetRegistry::findById(0x7BA, 0x7B0, &alphaA));
+    EXPECT_EQ(0x7B0, alphaA->getParentId());
+
+    ThingSetNode *alphaB = nullptr;
+    ASSERT_TRUE(ThingSetRegistry::findById(0x7BA, 0x7B1, &alphaB));
+    EXPECT_EQ(0x7B1, alphaB->getParentId());
+
+    EXPECT_NE(alphaA, alphaB) << "findById returned the same proxy for both parents";
+
+    // And again for the second shared id, to cover the neighbouring bucket
+    ThingSetNode *betaA = nullptr;
+    ASSERT_TRUE(ThingSetRegistry::findById(0x7BB, 0x7B0, &betaA));
+    EXPECT_EQ(0x7B0, betaA->getParentId());
+
+    ThingSetNode *betaB = nullptr;
+    ASSERT_TRUE(ThingSetRegistry::findById(0x7BB, 0x7B1, &betaB));
+    EXPECT_EQ(0x7B1, betaB->getParentId());
+
+    EXPECT_NE(betaA, betaB);
+}