diff --git a/subscriptionstore.cpp b/subscriptionstore.cpp index a0101a0..f82208a 100644 --- a/subscriptionstore.cpp +++ b/subscriptionstore.cpp @@ -61,6 +61,20 @@ void SubscriptionNode::removeSubscriber(const std::shared_ptr &subscrib } } +/** + * @brief SubscriptionNode::getChildren gets children or null pointer. Const, so doesn't default-create node for + * non-existing children. + * @param subtopic + * @return + */ +SubscriptionNode *SubscriptionNode::getChildren(const std::string &subtopic) const +{ + auto it = children.find(subtopic); + if (it != children.end()) + return it->second.get(); + return nullptr; +} + SubscriptionStore::SubscriptionStore() : root("root"), @@ -122,26 +136,25 @@ void SubscriptionStore::removeSubscription(std::shared_ptr &client, cons RWLockGuard lock_guard(&subscriptionsRwlock); lock_guard.wrlock(); - // TODO: because it's so similar to adding a subscription, make a function to retrieve the deepest node? + // This code looks like that for addSubscription(), but it's specifically different in that we don't want to default-create non-existing + // nodes. We need to abort when that happens. SubscriptionNode *deepestNode = &root; for(const std::string &subtopic : subtopics) { - std::unique_ptr *selectedChildren = nullptr; + SubscriptionNode *selectedChildren = nullptr; if (subtopic == "#") - selectedChildren = &deepestNode->childrenPound; + selectedChildren = deepestNode->childrenPound.get(); else if (subtopic == "+") - selectedChildren = &deepestNode->childrenPlus; + selectedChildren = deepestNode->childrenPlus.get(); else - selectedChildren = &deepestNode->children[subtopic]; - - std::unique_ptr &node = *selectedChildren; + selectedChildren = deepestNode->getChildren(subtopic); - if (!node) + if (!selectedChildren) { return; } - deepestNode = node.get(); + deepestNode = selectedChildren; } assert(deepestNode); @@ -237,6 +250,10 @@ void SubscriptionStore::publishRecursively(std::vector::const_itera return; } + // Null nodes in the tree shouldn't happen at this point. It points to bugs elsewhere. However, I don't remember why I check the pointer + // inside the if-block above, instead of the start of the method. + assert(this_node != nullptr); + if (this_node->children.empty() && !this_node->childrenPlus && !this_node->childrenPound) return; diff --git a/subscriptionstore.h b/subscriptionstore.h index b94d009..86ddee0 100644 --- a/subscriptionstore.h +++ b/subscriptionstore.h @@ -64,6 +64,8 @@ public: std::unique_ptr childrenPlus; std::unique_ptr childrenPound; + SubscriptionNode *getChildren(const std::string &subtopic) const; + int cleanSubscriptions(); };