Closes #22

core/tree_node.cpp

@@ -10,6 +10,7 @@
  */
 
 #include "tree_node.h"
+#include <stdexcept>
 
 TreeNode::TreeNode()
     : m_type(NodeType::Root), m_name("dbc") {
@@ -23,8 +24,14 @@ TreeNode::TreeNode(const SignalInfo &signal)
     : m_type(NodeType::Signal), m_name(signal.name), m_signal(std::make_unique<SignalInfo>(signal)) {
 }
 
-void TreeNode::AddChild(std::unique_ptr<TreeNode> child) {
+void TreeNode::AddChild(std::unique_ptr<TreeNode> &&child) {
     if (child) {
+        for (const TreeNode *ancestor = this; ancestor; ancestor = ancestor->m_parent) {
+            if (ancestor == child.get())
+                throw std::invalid_argument("Cannot insert a node into its own subtree");
+        }
+        if (child->m_parent)
+            throw std::invalid_argument("Child is already attached");
         m_children.push_back(std::move(child));
         m_children.back()->m_parent = this;
         m_children.back()->m_row = m_children.size() - 1U;
@@ -72,3 +79,18 @@ const TreeNode *TreeNode::GetParent() const {
 std::size_t TreeNode::GetRow() const {
     return m_row;
 }
+
+std::optional<FrameKey> TreeNode::GetFrameKey() const {
+    if (m_frame)
+        return FrameKey(m_frame->canId, m_frame->isExtended);
+    if (m_signal && m_parent && m_parent->GetType() == NodeType::Frame)
+        return m_parent->GetFrameKey();
+    return std::nullopt;
+}
+
+std::optional<SignalKey> TreeNode::GetSignalKey() const {
+    const std::optional<FrameKey> frame = GetFrameKey();
+    if (m_signal && frame)
+        return SignalKey(*frame, m_signal->name);
+    return std::nullopt;
+}

core/tree_node.h

@@ -14,9 +14,11 @@
 
 #include <cstddef>
 #include <memory>
+#include <optional>
 #include <string>
 #include <vector>
 
+#include "definition_key.h"
 #include "frame_info.h"
 #include "signal_info.h"
 
@@ -56,14 +58,19 @@ public:
     TreeNode(const TreeNode &) = delete;
     TreeNode &operator=(const TreeNode &) = delete;
 
+    // Node addresses are stable. Transfer unique_ptr ownership instead of nodes.
     TreeNode(TreeNode &&) = delete;
     TreeNode &operator=(TreeNode &&) = delete;
 
     /**
      * @brief Add child node.
      * @param child Child node to add.
+     * A null child is ignored. Existing children keep their addresses and rows.
+     * Ownership must be exclusive; attached nodes cannot be reparented.
+     * @throws std::invalid_argument for self/ancestor insertion or an attached child.
+     * Rejected ownership remains with the caller.
      */
-    void AddChild(std::unique_ptr<TreeNode> child);
+    void AddChild(std::unique_ptr<TreeNode> &&child);
 
     /** Non-owning parent; nullptr for a detached node or the root. */
     const TreeNode *GetParent() const;
@@ -71,6 +78,12 @@ public:
     /** Zero-based sibling position; zero for a detached node or the root. */
     std::size_t GetRow() const;
 
+    /** Frame identity for frame nodes and signals attached to a frame; otherwise empty. */
+    std::optional<FrameKey> GetFrameKey() const;
+
+    /** Signal identity (owning frame key and name); empty for other/detached nodes. */
+    std::optional<SignalKey> GetSignalKey() const;
+
     /**
      * @brief Get child count.
      * @return Number of children.

dbc.pro

@@ -1,5 +1,6 @@
 TEMPLATE = subdirs
-SUBDIRS += core tests
+SUBDIRS += core tests tree_tests
+tree_tests.file = tests/tree_tests.pro
 core.subdir = core
 tests.subdir = tests
 tests.depends = core

tests/tree_node_test.cpp

@@ -1,5 +1,6 @@
 // Standalone C++17 test: no Qt required.
 #include "dbc_tree_builder.h"
+#include <limits>
 #include <stdexcept>
 #include <type_traits>
 
@@ -7,27 +8,93 @@ static void Check(bool condition) {
     if (!condition) throw std::runtime_error("Tree navigation test failed");
 }
 
+static void CheckNavigation(const TreeNode &node) {
+    for (std::size_t row = 0; row < node.GetChildCount(); ++row) {
+        const TreeNode *child = node.GetChild(row);
+        Check(child->GetParent() == &node && child->GetRow() == row);
+        Check(child->GetParent()->GetChild(child->GetRow()) == child);
+        CheckNavigation(*child);
+    }
+    Check(!node.GetChild(node.GetChildCount()));
+    Check(!node.GetChild(std::numeric_limits<std::size_t>::max()));
+}
+
+static void CheckRejectedInsertion(TreeNode &parent, std::unique_ptr<TreeNode> &owner) {
+    const TreeNode *original = owner.get();
+    bool rejected = false;
+    try {
+        parent.AddChild(std::move(owner));
+    } catch (const std::invalid_argument &) {
+        rejected = true;
+    }
+    Check(rejected && owner.get() == original);
+    CheckNavigation(*owner);
+}
+
 int main() {
+    static_assert(!std::is_copy_constructible<TreeNode>::value, "Nodes have exclusive ownership");
+    static_assert(!std::is_copy_assignable<TreeNode>::value, "Nodes have exclusive ownership");
     static_assert(!std::is_move_constructible<TreeNode>::value, "Node addresses must be stable");
+    static_assert(!std::is_move_assignable<TreeNode>::value, "Node addresses must be stable");
+    CheckNavigation(*DbcTreeBuilder().Build(DbcDatabase()));
+
     DbcDatabase database;
     FrameInfo frame;
+    frame.canId = 123U;
+    frame.name = "Frame";
     SignalInfo signal;
+    signal.name = "Value";
     frame.signals.push_back(signal);
+    signal.name = "Other";
     frame.signals.push_back(signal);
     database.frames.push_back(frame);
+    frame.isExtended = true;
     database.frames.push_back(frame);
     std::unique_ptr<TreeNode> root = DbcTreeBuilder().Build(database);
     Check(root->GetParent() == nullptr && root->GetRow() == 0);
+    Check(!root->GetFrameKey() && !root->GetSignalKey());
     root->AddChild(nullptr);
     Check(root->GetChildCount() == 2 && !root->GetChild(2));
-    for (std::size_t row = 0; row < root->GetChildCount(); ++row) {
-        const TreeNode *child = root->GetChild(row);
-        Check(child->GetParent() == root.get() && child->GetRow() == row);
-        for (std::size_t position = 0; position < child->GetChildCount(); ++position) {
-            const TreeNode *leaf = child->GetChild(position);
-            Check(leaf->GetParent() == child && leaf->GetRow() == position);
-        }
+    Check(!root->GetChild(std::numeric_limits<std::size_t>::max()));
+    CheckNavigation(*root);
+
+    const TreeNode *firstFrame = root->GetChild(0);
+    const TreeNode *firstSignal = firstFrame->GetChild(0);
+    const FrameKey standard(123U, false);
+    const FrameKey extended(123U, true);
+    Check(firstFrame->GetFrameKey() == standard && !firstFrame->GetSignalKey());
+    Check(firstSignal->GetFrameKey() == standard);
+    Check(firstSignal->GetSignalKey() == SignalKey(standard, "Value"));
+    Check(firstFrame->GetChild(1)->GetSignalKey() == SignalKey(standard, "Other"));
+    Check(root->GetChild(1)->GetFrameKey() == extended);
+    Check(root->GetChild(1)->GetChild(0)->GetSignalKey() == SignalKey(extended, "Value"));
+
+    TreeNode detached(signal);
+    Check(!detached.GetFrameKey() && !detached.GetSignalKey());
+    Check(!detached.GetParent() && detached.GetRow() == 0);
+    TreeNode detachedFrame(frame);
+    Check(detachedFrame.GetFrameKey() == extended && !detachedFrame.GetSignalKey());
+
+    // Reordering/reloading source metadata does not change the original tree's keys.
+    database.frames.clear();
+    database.frames.push_back(frame);
+    std::unique_ptr<TreeNode> replacement = DbcTreeBuilder().Build(database);
+    Check(replacement->GetChild(0)->GetFrameKey() == root->GetChild(1)->GetFrameKey());
+    Check(firstSignal->GetSignalKey() == SignalKey(standard, "Value"));
+
+    // Grow both child vectors, retaining all existing addresses and rows.
+    for (std::size_t index = 0; index < 64; ++index) {
+        root->AddChild(std::make_unique<TreeNode>(frame));
+        root->GetChild(0)->AddChild(std::make_unique<TreeNode>(signal));
     }
+    Check(root->GetChild(0) == firstFrame && firstFrame->GetChild(0) == firstSignal);
+    CheckNavigation(*root);
+    CheckRejectedInsertion(*root, root);
+    CheckRejectedInsertion(*root->GetChild(0)->GetChild(0), root);
+
+    const TreeNode *original = root.get();
     std::unique_ptr<TreeNode> transferred = std::move(root);
-    Check(transferred->GetChild(0)->GetParent() == transferred.get());
+    Check(!root && transferred.get() == original);
+    CheckNavigation(*transferred);
+    Check(firstSignal->GetSignalKey() == SignalKey(standard, "Value"));
 }

tests/tree_tests.pro

@@ -0,0 +1,6 @@
+TEMPLATE = app
+TARGET = dbc_tree_tests
+CONFIG += console testcase c++17
+CONFIG -= qt app_bundle debug_and_release debug_and_release_target
+INCLUDEPATH += ../core
+SOURCES += tree_node_test.cpp ../core/tree_node.cpp ../core/dbc_tree_builder.cpp