diff --git a/include/grove/IDataNode.h b/include/grove/IDataNode.h index cb39c01..3b80de7 100644 --- a/include/grove/IDataNode.h +++ b/include/grove/IDataNode.h @@ -25,12 +25,21 @@ public: // ======================================== /** - * @brief Get direct child by name + * @brief Get direct child by name (transfers ownership) * @param name Exact name of the child * @return Child node or nullptr if not found + * @warning This removes the child from the tree. Use getChildReadOnly() for non-destructive reads. */ virtual std::unique_ptr getChild(const std::string& name) = 0; + /** + * @brief Get direct child by name (read-only, no ownership transfer) + * @param name Exact name of the child + * @return Raw pointer to child node or nullptr if not found + * @note The returned pointer is valid as long as the parent node exists and the child isn't removed. + */ + virtual IDataNode* getChildReadOnly(const std::string& name) = 0; + /** * @brief Get names of all direct children * @return Vector of child names diff --git a/include/grove/IDataTree.h b/include/grove/IDataTree.h index 3cabd0b..51d9040 100644 --- a/include/grove/IDataTree.h +++ b/include/grove/IDataTree.h @@ -57,11 +57,21 @@ public: /** * @brief Get persistent data root (read-write, saved to disk) * @return Data root node (data/) + * @warning Creates a copy. Use getDataRootReadOnly() for non-destructive reads. * * Use for: Campaign progress, unlocks, player statistics */ virtual std::unique_ptr getDataRoot() = 0; + /** + * @brief Get persistent data root (read-only, no copy) + * @return Raw pointer to data root node (data/) + * @note Pointer is valid as long as the tree exists + * + * Use for: Reading data without making copies + */ + virtual IDataNode* getDataRootReadOnly() = 0; + /** * @brief Get runtime data root (read-write, never saved) * @return Runtime root node (runtime/) diff --git a/include/grove/JsonDataNode.h b/include/grove/JsonDataNode.h index 2f4e63a..d092e60 100644 --- a/include/grove/JsonDataNode.h +++ b/include/grove/JsonDataNode.h @@ -39,6 +39,7 @@ public: // Tree navigation std::unique_ptr getChild(const std::string& name) override; + IDataNode* getChildReadOnly(const std::string& name) override; std::vector getChildNames() override; bool hasChildren() override; diff --git a/include/grove/JsonDataTree.h b/include/grove/JsonDataTree.h index 52257e6..b48dbca 100644 --- a/include/grove/JsonDataTree.h +++ b/include/grove/JsonDataTree.h @@ -47,6 +47,7 @@ public: // Separate roots std::unique_ptr getConfigRoot() override; std::unique_ptr getDataRoot() override; + IDataNode* getDataRootReadOnly() override; std::unique_ptr getRuntimeRoot() override; // Save operations diff --git a/src/JsonDataNode.cpp b/src/JsonDataNode.cpp index 3f8a58e..a490886 100644 --- a/src/JsonDataNode.cpp +++ b/src/JsonDataNode.cpp @@ -27,6 +27,15 @@ std::unique_ptr JsonDataNode::getChild(const std::string& name) { it->second->m_readOnly); } +IDataNode* JsonDataNode::getChildReadOnly(const std::string& name) { + auto it = m_children.find(name); + if (it == m_children.end()) { + return nullptr; + } + // Return raw pointer without copying - valid as long as parent exists + return it->second.get(); +} + std::vector JsonDataNode::getChildNames() { std::vector names; names.reserve(m_children.size()); diff --git a/src/JsonDataTree.cpp b/src/JsonDataTree.cpp index 274d217..4e2d566 100644 --- a/src/JsonDataTree.cpp +++ b/src/JsonDataTree.cpp @@ -88,6 +88,11 @@ std::unique_ptr JsonDataTree::getDataRoot() { false); } +IDataNode* JsonDataTree::getDataRootReadOnly() { + // Return raw pointer without copying - valid as long as tree exists + return m_root->getFirstChildByName("data"); +} + std::unique_ptr JsonDataTree::getRuntimeRoot() { auto runtimeNode = m_root->getFirstChildByName("runtime"); if (!runtimeNode) { diff --git a/tests/integration/test_13_cross_system.cpp b/tests/integration/test_13_cross_system.cpp index 5673176..d3a933d 100644 --- a/tests/integration/test_13_cross_system.cpp +++ b/tests/integration/test_13_cross_system.cpp @@ -186,14 +186,17 @@ int main() { messagesReceived++; std::cout << " EconomyModule received: " << msg.topic << "\n"; - // Read player data from tree - auto playerData = tree->getDataRoot()->getChild("player"); - if (playerData) { - auto profileData = playerData->getChild("profile"); - if (profileData) { - int gold = profileData->getInt("gold"); - std::cout << " Player gold: " << gold << "\n"; - ASSERT_EQ(gold, 1000, "Gold should match saved value"); + // Read player data from tree using read-only access + auto dataRoot = tree->getDataRoot(); + if (dataRoot) { + auto playerData = dataRoot->getChildReadOnly("player"); + if (playerData) { + auto profileData = playerData->getChildReadOnly("profile"); + if (profileData) { + int gold = profileData->getInt("gold"); + std::cout << " Player gold: " << gold << "\n"; + ASSERT_EQ(gold, 1000, "Gold should match saved value"); + } } } } @@ -211,14 +214,16 @@ int main() { int syncErrors = 0; for (int i = 0; i < 10; i++) { - // Update gold in DataNode + // Update gold in DataNode using read-only access int goldValue = 1000 + i * 10; - auto playerNode = tree->getDataRoot()->getChild("player"); - if (playerNode) { - auto profileNode = playerNode->getChild("profile"); - if (profileNode) { - profileNode->setInt("gold", goldValue); - // Note: Changes are applied directly, no need to move nodes back + auto dataRoot = tree->getDataRoot(); + if (dataRoot) { + auto playerNode = dataRoot->getChildReadOnly("player"); + if (playerNode) { + auto profileNode = playerNode->getChildReadOnly("profile"); + if (profileNode) { + profileNode->setInt("gold", goldValue); + } } } @@ -236,16 +241,19 @@ int main() { auto msg = economyIO->pullMessage(); int msgGold = msg.data->getInt("gold"); - // Read from DataNode - auto playerCheck = tree->getDataRoot()->getChild("player"); - if (playerCheck) { - auto profileCheck = playerCheck->getChild("profile"); - if (profileCheck) { - int dataGold = profileCheck->getInt("gold"); + // Read from DataNode using read-only access + auto dataRoot = tree->getDataRoot(); + if (dataRoot) { + auto playerCheck = dataRoot->getChildReadOnly("player"); + if (playerCheck) { + auto profileCheck = playerCheck->getChildReadOnly("profile"); + if (profileCheck) { + int dataGold = profileCheck->getInt("gold"); - if (msgGold != dataGold) { - std::cerr << " SYNC ERROR: msg=" << msgGold << " data=" << dataGold << "\n"; - syncErrors++; + if (msgGold != dataGold) { + std::cerr << " SYNC ERROR: msg=" << msgGold << " data=" << dataGold << "\n"; + syncErrors++; + } } } } @@ -306,12 +314,31 @@ int main() { std::cout << "✓ TEST 4 PASSED\n"; // ======================================================================== - // TEST 5: Concurrent Access (IO Only - DataNode concurrent reads not supported yet) + // TEST 5: Concurrent Access (IO + DataNode with new read-only API) // ======================================================================== - std::cout << "\n=== TEST 5: Concurrent Access (IO Publishing) ===\n"; + std::cout << "\n=== TEST 5: Concurrent Access (IO + DataNode) ===\n"; + + // Recreate player data for TEST 5 (ensure fresh data exists for concurrent reads) + auto player5 = std::make_unique("player", nlohmann::json::object()); + auto profile5 = std::make_unique("profile", nlohmann::json{ + {"name", "TestPlayer"}, + {"level", 6}, + {"gold", 1090} // Final gold value from TEST 3 + }); + player5->setChild("profile", std::move(profile5)); + + // Use getDataRootReadOnly() to get the actual root (not a copy) and add the child + auto dataRootPtr = tree->getDataRootReadOnly(); + if (dataRootPtr) { + dataRootPtr->setChild("player", std::move(player5)); + std::cout << " Player data recreated for concurrent test\n"; + } else { + std::cerr << " ERROR: Could not get data root!\n"; + } std::atomic running{true}; std::atomic publishCount{0}; + std::atomic readCount{0}; std::atomic errors{0}; // Thread 1: Publish events @@ -327,23 +354,48 @@ int main() { } }); - // Note: Concurrent DataNode reads are not supported because getDataRoot() transfers ownership. - // A future API improvement would add getDataRootReadOnly() -> IDataNode* for non-destructive reads. + // Thread 2: Read DataNode concurrently using new read-only API + std::thread readThread([&]() { + while (running) { + try { + // Use getDataRootReadOnly() instead of getDataRoot() to avoid copying + auto dataRoot = tree->getDataRootReadOnly(); + if (dataRoot) { + // Use getChildReadOnly() instead of getChild() to avoid copying/ownership transfer + auto playerData = dataRoot->getChildReadOnly("player"); + if (playerData) { + auto profileData = playerData->getChildReadOnly("profile"); + if (profileData) { + int gold = profileData->getInt("gold", 0); + readCount++; + } + } + } + std::this_thread::sleep_for(std::chrono::milliseconds(20)); + } catch (...) { + errors++; + } + } + }); // Run for 2 seconds std::this_thread::sleep_for(std::chrono::seconds(2)); running = false; pubThread.join(); + readThread.join(); std::cout << "Concurrent test completed:\n"; std::cout << " Publishes: " << publishCount << "\n"; + std::cout << " Reads: " << readCount << "\n"; std::cout << " Errors: " << errors << "\n"; ASSERT_EQ(errors.load(), 0, "Should have zero exceptions during concurrent access"); ASSERT_GT(publishCount.load(), 0, "Should have published messages"); + ASSERT_GT(readCount.load(), 0, "Should have successfully read DataNode data"); reporter.addMetric("concurrent_publishes", publishCount); + reporter.addMetric("concurrent_reads", readCount); reporter.addMetric("concurrent_errors", errors); reporter.addAssertion("concurrent_access", errors == 0); std::cout << "✓ TEST 5 PASSED\n";