Fix and cleanup retained map test

test/retained_topic_map.cpp

@@ -3,6 +3,7 @@
 // Distributed under the Boost Software License, Version 1.0.
 // (See accompanying file LICENSE_1_0.txt or copy at
 // http://www.boost.org/LICENSE_1_0.txt)
+
 #include "test_main.hpp"
 #include "combi_test.hpp"
 #include "checker.hpp"
@@ -11,6 +12,8 @@
 #include "retained_topic_map.hpp"
 
 #include <boost/format.hpp>
+#include <algorithm>
+#include <random>
 
 BOOST_AUTO_TEST_SUITE(test_retained_map)
 
@@ -206,39 +209,49 @@ BOOST_AUTO_TEST_CASE(erase_upper_first) {
     }
 }
 
-#if 0
-
 BOOST_AUTO_TEST_CASE(large_number_of_topics) {
-    retained_topic_map<std::size_t> map;
+    retained_topic_map<std::pair<std::size_t, std::size_t>> map;
 
-    constexpr std::size_t num_topics = 10000;
+    std::vector< std::pair<std::string, std::pair<std::size_t, std::size_t> > > created_topics;
+
+    constexpr std::size_t num_topics = 25;
     for (std::size_t i = 0; i < num_topics; ++i) {
-        map.insert_or_assign((boost::format("topic/%d") % i).str(), i);
-    }
+        for (std::size_t j = 0; j < num_topics; ++j) {
+            std::string topic = (boost::format("topic/first_level_%d/second_level_%d") % i % j).str();
+            auto value = std::make_pair(i, j);
 
-    BOOST_TEST(map.size() == num_topics);
-    try {
-        for (std::size_t i = 0; i < num_topics; ++i) {
-            map.find(
-                (boost::format("topic/%d") % i).str(),
-                [&](std::size_t value) {
-                    if (value != i) throw false;
-                }
-            );
+            map.insert_or_assign(topic, value);
+
+            created_topics.push_back(std::make_pair(topic, value));
         }
     }
-    catch (bool) {
-        BOOST_TEST(false);
+
+    BOOST_TEST(map.size() == num_topics * num_topics);
+
+    std::vector< std::pair<std::size_t, std::size_t> > received_values;
+    std::vector< std::pair<std::size_t, std::size_t> > searched_values;
+
+    std::shuffle(created_topics.begin(), created_topics.end(), std::default_random_engine(0x12345));
+
+    for (auto const& i : created_topics) {
+       map.find(
+            i.first,
+            [&](std::pair<std::size_t, std::size_t> const& value) {
+                received_values.push_back(value);
+            }
+        );
+
+        searched_values.push_back(i.second);
     }
 
-    for (std::size_t i = 0; i < num_topics; ++i) {
-        map.erase((boost::format("topic/%d") % i).str());
+    BOOST_TEST(searched_values == received_values);
+
+    for (auto const& i : created_topics) {
+        map.erase(i.first);
     }
 
     BOOST_TEST(map.size() == 0);
     BOOST_TEST(map.internal_size() == 1);
 }
 
-#endif
-
 BOOST_AUTO_TEST_SUITE_END()

test/retained_topic_map.hpp

@@ -23,6 +23,11 @@ namespace mi = boost::multi_index;
 
 template<typename Value>
 class retained_topic_map {
+
+    // Exceptions used
+    static void throw_max_stored_topics() { throw std::overflow_error("Retained map maximum number of topics reached"); }
+    static void throw_no_wildcards_allowed() { throw std::runtime_error("Retained map no wildcards allowed in retained topic name"); }
+
     using node_id_t = std::size_t;
 
     static constexpr node_id_t root_parent_id = 0;
@@ -37,9 +42,23 @@ class retained_topic_map {
         node_id_t id;
 
         std::size_t count = 1;
-
         static constexpr std::size_t max_count = std::numeric_limits<std::size_t>::max();
 
+        // Increase the count for this node
+        void increase_count() {
+            if (count == max_count) {
+                throw_max_stored_topics();
+            }
+
+            ++count;
+        }
+
+        // Decrease the count for this node
+        void decrease_count() {
+            BOOST_ASSERT(count >= count);
+            --count;
+        }
+
         MQTT_NS::optional<Value> value;
 
         path_entry(node_id_t parent_id, MQTT_NS::string_view name, node_id_t id)
@@ -98,7 +117,7 @@ class retained_topic_map {
                     }
                 }
                 else {
-                    direct_index.modify(entry, [](path_entry &entry){ ++entry.count; });
+                    direct_index.modify(entry, [](path_entry& entry){ entry.increase_count(); });
                 }
 
                 parent = entry;
@@ -230,7 +249,7 @@ class retained_topic_map {
 
             // Do iterators stay valid when erasing ? I think they do ?
             for (auto entry : path) {
-                direct_index.modify(entry, [](path_entry &entry){ --entry.count; });
+                direct_index.modify(entry, [](path_entry& entry){ entry.decrease_count(); });
 
                 if (entry->count == 0) {
                     map.erase(entry);
@@ -248,14 +267,10 @@ class retained_topic_map {
         auto& direct_index = map.template get<direct_index_tag>();
 
         for(auto& i : path) {
-            direct_index.modify(i, [](path_entry &entry){ ++entry.count; });
+            direct_index.modify(i, [](path_entry& entry){ entry.increase_count(); });
         }
     }
 
-    // Exceptions used
-    static void throw_max_stored_topics() { throw std::overflow_error("Retained map maximum number of topics reached"); }
-    static void throw_no_wildcards_allowed() { throw std::runtime_error("Retained map no wildcards allowed in retained topic name"); }
-
     // Increase the map size (total number of topics stored)
     void increase_map_size() {
         if(map_size == std::numeric_limits<decltype(map_size)>::max()) {

test/subscription_map.hpp

@@ -202,6 +202,12 @@ class subscription_map_base {
         count.increment_value();
     }
 
+    // Decrease the subscription count for a specific node
+    static void decrease_count_storage(count_storage_t& count) {
+        BOOST_ASSERT(count.value() > 0);
+        count.decrement_value();
+    }
+
     using this_type = subscription_map_base<Value>;
 
     // Use boost hash to hash pair in path_entry_key
@@ -311,7 +317,7 @@ class subscription_map_base {
                 remove_hash_child_flag = false;
             }
 
-            entry->second.count.decrement_value();
+            decrease_count_storage(entry->second.count);
             if (entry->second.count.value() == 0) {
                 remove_plus_child_flag = (entry->first.second == "+");
                 remove_hash_child_flag = (entry->first.second == "#");