KEP-486 Made change requested in review

bluzelle · Oct 4, 2018 · f7f7d64 · f7f7d64
1 parent fa50467
commit f7f7d64
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 87 deletions.
diff --git a/raft/raft.cpp b/raft/raft.cpp
@@ -21,7 +21,6 @@
 #include <proto/bluzelle.pb.h>
 #include <utils/crypto.hpp>
 #include <utils/blacklist.hpp>
-#include <utils/container.hpp>
 
 namespace
 {
@@ -35,6 +34,22 @@ namespace
     const std::chrono::milliseconds  DEFAULT_ELECTION_TIMER_LEN{std::chrono::milliseconds(1250)};
 
     const std::string RAFT_TIMEOUT_SCALE = "RAFT_TIMEOUT_SCALE";
+
+    std::mt19937 gen(std::time(0)); //Standard mersenne_twister_engine seeded with rd()
+
+    // TODO: RHN - this should be templatized
+    bzn::peers_list_t::const_iterator
+    choose_any_one_of(const bzn::peers_list_t& all_peers)
+    {
+        if (all_peers.size()>0)
+        {
+            std::uniform_int_distribution<> dis(1, all_peers.size());
+            auto it = all_peers.begin();
+            std::advance(it, dis(gen) - 1);
+            return it;
+        }
+        return all_peers.end();
+    }
 }
 
 
@@ -1441,7 +1456,7 @@ raft::auto_add_peer_if_required()
                 {
                     throw std::runtime_error(ERROR_BOOTSTRAP_LIST_MUST_HAVE_MORE_THAN_ONE_PEER);
                 }
-                return *bzn::utils::container::choose_any_one_of(other_peers);
+                return *choose_any_one_of(other_peers);
             };
 
     bzn::peer_address_t leader{((this->get_leader_unsafe().uuid.empty() && this->get_leader_unsafe().host.empty()) ?  choose_leader() : this->get_leader_unsafe())};

diff --git a/utils/CMakeLists.txt b/utils/CMakeLists.txt
@@ -6,8 +6,7 @@ add_library(utils STATIC
         make_endpoint.hpp
         make_endpoint.cpp
         crypto.cpp
-        crypto.hpp
-        container.hpp)
+        crypto.hpp)
 
 target_link_libraries(utils ${CURL_LIBRARIES} ${JSONCPP_LIBRARIES} OpenSSL::SSL)
 

diff --git a/utils/container.hpp b/utils/container.hpp
diff --git a/utils/test/utils_test.cpp b/utils/test/utils_test.cpp
@@ -19,7 +19,6 @@
 #include <boost/beast/core/detail/base64.hpp>
 #include <utils/blacklist.hpp>
 #include <utils/crypto.hpp>
-#include <utils/container.hpp>
 
 using namespace::testing;
 
@@ -329,43 +328,3 @@ TEST(util_test, test_that_verifying_asignature_with_empty_inputs_will_fail_grace
     EXPECT_FALSE(bzn::utils::crypto::verify_signature( public_pem, "", valid_uuid));
     EXPECT_FALSE(bzn::utils::crypto::verify_signature( public_pem, signature, ""));
 }
-
-
-TEST(util_test, test_that_choose_any_one_of_chooses_one_of_a_set)
-{
-    const bzn::uuid_t LEADER_NODE_UUID  {"fb300a30-49fd-4230-8044-0e3069948e42"};
-    const bzn::uuid_t TEST_NODE_UUID    {"f0645cc2-476b-485d-b589-217be3ca87d5"};
-    const bzn::uuid_t FOLLOWER_NODE_UUID{"8993098f-e32e-4b6f-9db9-c770c9bc2509"};
-    const bzn::peers_list_t TEST_LIST { // using http_port as an index.
-        {"127.0.0.1", 8081, 0, "leader",   LEADER_NODE_UUID},
-        {"127.0.0.2", 8082, 1, "follower", FOLLOWER_NODE_UUID},
-        {"127.0.0.3", 8083, 2, "sut",      TEST_NODE_UUID},
-        {"127.0.0.4", 8084, 3, "sut0",     TEST_NODE_UUID},
-        {"127.0.0.5", 8085, 4, "sut1",     TEST_NODE_UUID},
-        {"127.0.0.6", 8086, 5, "sut2",     TEST_NODE_UUID},
-        {"127.0.0.7", 8087, 6, "sut3",     TEST_NODE_UUID},
-        {"127.0.0.8", 8088, 7, "sut4",     TEST_NODE_UUID}};
-
-    // How do you test a function that uses random?? Well I just want to 
-    // make sure that every element in the array gets chosen at some point, 
-    // and that the function is unlikely to fail.
-    std::array<bool, 8> histogram{false};
-    size_t count{0};
-    while( static_cast<size_t>(std::count_if(histogram.begin(), histogram.end(), [](bool i){return i;})) < TEST_LIST.size())
-    {
-        auto peer = bzn::utils::container::choose_any_one_of(TEST_LIST);
-        EXPECT_TRUE(peer != TEST_LIST.end());
-        EXPECT_TRUE( TEST_LIST.end() != std::find_if( TEST_LIST.begin(), TEST_LIST.end(), [peer](bzn::peer_address_t addr){ return addr.port == peer->port; }));
-        histogram[peer->http_port] = true;
-        EXPECT_TRUE(10000 > ++count);
-    }
-    // this is redundant, I know.
-    EXPECT_EQ(8, std::count_if(histogram.begin(), histogram.end(), [](bool i){return i;}));
-}
-
-
-TEST(util_test, test_that_choose_any_one_of_behaves_when_given_bad_input)
-{
-    const bzn::peers_list_t TEST_LIST {};
-    EXPECT_EQ(bzn::utils::container::choose_any_one_of(TEST_LIST), TEST_LIST.end());
-}