StillHammer/GroveEngine
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat: Add read-only API for concurrent DataNode access & restore test_13 cross-system tests

Browse Source 
PROBLEM: test_13 "Cross-System Integration" had concurrent DataNode reads removed because
getChild() and getDataRoot() return unique_ptr (ownership transfer), making concurrent
reads impossible - each read would create a copy or destroy the data.

SOLUTION: Add read-only API methods that return raw pointers without copying:

API Changes:
1. **IDataNode::getChildReadOnly(name)** → IDataNode*
   - Returns raw pointer to child without copying
   - Pointer valid as long as parent exists
   - Enables concurrent reads without destroying tree

2. **IDataTree::getDataRootReadOnly()** → IDataNode*
   - Returns raw pointer to data root without copying
   - Enables concurrent access to tree data
   - Complements existing getDataRoot() which returns copy

3. **JsonDataNode::getChildReadOnly()** implementation
   - Returns m_children[name].get() directly
   - Zero-overhead, no allocation

4. **JsonDataTree::getDataRootReadOnly()** implementation
   - Returns m_root->getFirstChildByName("data") directly
   - No copying, direct access

Test Changes:
- Restored TEST 5 concurrent access with IO + DataNode
- Uses getDataRootReadOnly() + getChildReadOnly() for reads
- Thread 1: Publishes IO messages concurrently
- Thread 2: Reads DataNode data concurrently (NOW WORKS!)
- Updated TEST 2 & 3 to use read-only API where appropriate
- Recreate player data before TEST 5 using read-only root access

Results:
✅ test_13 ALL TESTS PASS (5/5)
✅ TEST 5: ~100 concurrent reads successful (was 0 before)
✅ 0 errors during concurrent access
✅ True cross-system integration validated (IO + DataNode together)

This restores the original purpose of test_13: validating that IO pub/sub
and DataNode tree access work correctly together in concurrent scenarios.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
StillHammer 2025-11-20 14:02:06 +08:00
parent 113b966341
commit 31031804ba
7 changed files with 116 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
include/grove/IDataNode.h
View File

@ -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<IDataNode> 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

10
include/grove/IDataTree.h
View File

@ -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<IDataNode> 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/)

1
include/grove/JsonDataNode.h
View File

@ -39,6 +39,7 @@ public:
// Tree navigation
std::unique_ptr<IDataNode> getChild(const std::string& name) override;
IDataNode* getChildReadOnly(const std::string& name) override;
std::vector<std::string> getChildNames() override;
bool hasChildren() override;

1
include/grove/JsonDataTree.h
View File

@ -47,6 +47,7 @@ public:
// Separate roots
std::unique_ptr<IDataNode> getConfigRoot() override;
std::unique_ptr<IDataNode> getDataRoot() override;
IDataNode* getDataRootReadOnly() override;
std::unique_ptr<IDataNode> getRuntimeRoot() override;
// Save operations

9
src/JsonDataNode.cpp
View File

@ -27,6 +27,15 @@ std::unique_ptr<IDataNode> 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<std::string> JsonDataNode::getChildNames() {
std::vector<std::string> names;
names.reserve(m_children.size());

5
src/JsonDataTree.cpp
View File

@ -88,6 +88,11 @@ std::unique_ptr<IDataNode> 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<IDataNode> JsonDataTree::getRuntimeRoot() {
auto runtimeNode = m_root->getFirstChildByName("runtime");
if (!runtimeNode) {

108
tests/integration/test_13_cross_system.cpp
View File

@ -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<JsonDataNode>("player", nlohmann::json::object());
auto profile5 = std::make_unique<JsonDataNode>("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<bool> running{true};
std::atomic<int> publishCount{0};
std::atomic<int> readCount{0};
std::atomic<int> 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";