Provide create function for ordering creation

Signed-off-by: kamilsa <[email protected]>

irohad/consensus/yac/cluster_order.hpp

@@ -31,7 +31,13 @@ namespace iroha {
       class ClusterOrdering {
        public:
 
-        explicit ClusterOrdering(std::vector<model::Peer> order);
+        /**
+         * Creates cluster ordering from the vector of peers
+         * @param order vector of peers
+         * @return false if vector is empty, true otherwise
+         */
+        static nonstd::optional<ClusterOrdering> create(
+            std::vector<model::Peer> order);
 
         /**
          * Provide current leader peer
@@ -49,13 +55,21 @@ namespace iroha {
          */
         bool hasNext();
 
-        std::vector<model::Peer> getPeers() { return order_; };
+        std::vector<model::Peer> getPeers() {
+          return order_;
+        };
 
-        auto getNumberOfPeers() { return order_.size(); }
+        auto getNumberOfPeers() {
+          return order_.size();
+        }
 
         virtual ~ClusterOrdering() = default;
 
        private:
+        // prohibit creation of the object not from create method
+        explicit ClusterOrdering(std::vector<model::Peer> order);
+        ClusterOrdering() = delete;
+
         std::vector<model::Peer> order_;
         uint32_t index_ = 0;
       };

irohad/consensus/yac/impl/cluster_order.cpp

@@ -15,19 +15,26 @@
  * limitations under the License.
  */
 
-#include <utility>
 #include "consensus/yac/cluster_order.hpp"
 
 namespace iroha {
   namespace consensus {
     namespace yac {
 
+      nonstd::optional<ClusterOrdering> ClusterOrdering::create(
+          std::vector<model::Peer> order) {
+        if (order.empty()) {
+          return nonstd::nullopt;
+        }
+        return ClusterOrdering(order);
+      }
+
       ClusterOrdering::ClusterOrdering(std::vector<model::Peer> order)
           : order_(std::move(order)) {}
 
       model::Peer ClusterOrdering::currentLeader() {
         if (index_ >= order_.size()) {
-          index_ = 0;  // TODO 01/08/17 Muratov: dangerous indexing, what if order_.size == 0? IR-503
+          index_ = 0;
         }
         return order_.at(index_);
       }

irohad/consensus/yac/impl/peer_orderer_impl.cpp

@@ -27,7 +27,7 @@ namespace iroha {
 
       nonstd::optional<ClusterOrdering> PeerOrdererImpl::getInitialOrdering() {
         return query_->getLedgerPeers() | [](const auto &peers) {
-          return nonstd::make_optional<ClusterOrdering>(peers);
+          return ClusterOrdering::create(peers);
         };
       }
 

test/integration/consensus/consensus_sunny_day.cpp

@@ -67,11 +67,14 @@ class ConsensusSunnyDayTest : public ::testing::Test {
     network = std::make_shared<NetworkImpl>();
     crypto = std::make_shared<FixedCryptoProvider>(std::to_string(my_num));
     timer = std::make_shared<TimerImpl>();
+    auto order = ClusterOrdering::create(default_peers);
+    ASSERT_TRUE(order);
+
     yac = Yac::create(YacVoteStorage(),
                       network,
                       crypto,
                       timer,
-                      ClusterOrdering(default_peers),
+                      order.value(),
                       delay);
     network->subscribe(yac);
 
@@ -143,7 +146,11 @@ TEST_F(ConsensusSunnyDayTest, SunnyDayTest) {
   std::this_thread::sleep_for(std::chrono::milliseconds(delay_before));
 
   YacHash my_hash("proposal_hash", "block_hash");
-  yac->vote(my_hash, ClusterOrdering(default_peers));
+
+  auto order = ClusterOrdering::create(default_peers);
+  ASSERT_TRUE(order.has_value());
+
+  yac->vote(my_hash, order.value());
   std::this_thread::sleep_for(std::chrono::milliseconds(delay_after));
 
   ASSERT_TRUE(wrapper.validate());

test/module/irohad/consensus/yac/cluster_order_test.cpp

@@ -24,8 +24,9 @@ TEST(ClusterOrderTest, ClusterOrderOnNext) {
   iroha::model::Peer p2;
   p2.address = "2";
   std::vector<iroha::model::Peer> peers = {p1, p2};
-  iroha::consensus::yac::ClusterOrdering order(peers);
-  ASSERT_EQ("1", order.currentLeader().address);
-  ASSERT_EQ("2", order.switchToNext().currentLeader().address);
-  ASSERT_EQ("1", order.switchToNext().currentLeader().address);
+  auto order = iroha::consensus::yac::ClusterOrdering::create(peers);
+  ASSERT_TRUE(order);
+  ASSERT_EQ("1", order->currentLeader().address);
+  ASSERT_EQ("2", order->switchToNext().currentLeader().address);
+  ASSERT_EQ("1", order->switchToNext().currentLeader().address);
 }

test/module/irohad/consensus/yac/yac_gate_test.cpp

@@ -89,7 +89,7 @@ TEST_F(YacGateTest, YacGateSubscriptionTest) {
 
   // generate order of peers
   EXPECT_CALL(*peer_orderer, getOrdering(_))
-      .WillOnce(Return(ClusterOrdering({mk_peer("fake_node")})));
+      .WillOnce(Return(ClusterOrdering::create({mk_peer("fake_node")})));
 
   // make hash from block
   EXPECT_CALL(*hash_provider, makeHash(_)).WillOnce(Return(expected_hash));
@@ -143,7 +143,7 @@ TEST_F(YacGateTest, LoadBlockWhenDifferentCommit) {
 
   // generate order of peers
   EXPECT_CALL(*peer_orderer, getOrdering(_))
-      .WillOnce(Return(ClusterOrdering({mk_peer("fake_node")})));
+      .WillOnce(Return(ClusterOrdering::create({mk_peer("fake_node")})));
 
   EXPECT_CALL(*hash_gate, vote(expected_hash, _)).Times(1);
 

test/module/irohad/consensus/yac/yac_mocks.hpp

@@ -39,8 +39,8 @@ namespace iroha {
       VoteMessage create_vote(YacHash hash, std::string pub_key) {
         VoteMessage vote;
         vote.hash = hash;
-        std::copy(pub_key.begin(), pub_key.end(),
-                  vote.signature.pubkey.begin());
+        std::copy(
+            pub_key.begin(), pub_key.end(), vote.signature.pubkey.begin());
         return vote;
       }
 
@@ -78,7 +78,9 @@ namespace iroha {
 
         MockTimer(const MockTimer &rhs) {}
 
-        MockTimer &operator=(const MockTimer &rhs) { return *this; }
+        MockTimer &operator=(const MockTimer &rhs) {
+          return *this;
+        }
       };
 
       class MockYacNetwork : public YacNetwork {
@@ -88,7 +90,9 @@ namespace iroha {
           notification = handler;
         };
 
-        void release() { notification.reset(); }
+        void release() {
+          notification.reset();
+        }
 
         MOCK_METHOD2(send_commit, void(model::Peer, CommitMessage));
         MOCK_METHOD2(send_reject, void(model::Peer, RejectMessage));
@@ -128,7 +132,9 @@ namespace iroha {
 
         MockHashGate(MockHashGate &&rhs) {}
 
-        MockHashGate &operator=(const MockHashGate &rhs) { return *this; };
+        MockHashGate &operator=(const MockHashGate &rhs) {
+          return *this;
+        };
       };
 
       class MockYacPeerOrderer : public YacPeerOrderer {
@@ -139,9 +145,9 @@ namespace iroha {
 
         MockYacPeerOrderer() = default;
 
-        MockYacPeerOrderer(const MockYacPeerOrderer &rhs) {};
+        MockYacPeerOrderer(const MockYacPeerOrderer &rhs){};
 
-        MockYacPeerOrderer(MockYacPeerOrderer &&rhs) {};
+        MockYacPeerOrderer(MockYacPeerOrderer &&rhs){};
 
         MockYacPeerOrderer &operator=(const MockYacPeerOrderer &rhs) {
           return *this;
@@ -157,9 +163,9 @@ namespace iroha {
 
         MockYacHashProvider() = default;
 
-        MockYacHashProvider(const MockYacHashProvider &rhs) {};
+        MockYacHashProvider(const MockYacHashProvider &rhs){};
 
-        MockYacHashProvider(MockYacHashProvider &&rhs) {};
+        MockYacHashProvider(MockYacHashProvider &&rhs){};
 
         MockYacHashProvider &operator=(const MockYacHashProvider &rhs) {
           return *this;
@@ -195,12 +201,16 @@ namespace iroha {
           network = std::make_shared<MockYacNetwork>();
           crypto = std::make_shared<MockYacCryptoProvider>();
           timer = std::make_shared<MockTimer>();
-          yac = Yac::create(YacVoteStorage(), network, crypto, timer,
-                            ClusterOrdering(default_peers), delay);
+          auto ordering = ClusterOrdering::create(default_peers);
+          ASSERT_TRUE(ordering.has_value());
+          yac = Yac::create(
+              YacVoteStorage(), network, crypto, timer, ordering.value(), delay);
           network->subscribe(yac);
         };
 
-        void TearDown() override { network->release(); };
+        void TearDown() override {
+          network->release();
+        };
       };
     }  // namespace yac
   }    // namespace consensus

test/module/irohad/consensus/yac/yac_simple_cold_case_test.cpp

@@ -51,11 +51,14 @@ TEST_F(YacTest, YacWhenInit) {
 
   auto fake_delay_ = 100500;
 
+  auto order = ClusterOrdering::create(default_peers);
+  ASSERT_TRUE(order.has_value());
+
   auto yac_ = Yac::create(YacVoteStorage(),
                           std::make_shared<MockYacNetwork>(network_),
                           std::make_shared<MockYacCryptoProvider>(crypto_),
                           std::make_shared<MockTimer>(timer_),
-                          ClusterOrdering(default_peers),
+                          order.value(),
                           fake_delay_);
 
   network_.subscribe(yac_);
@@ -72,7 +75,11 @@ TEST_F(YacTest, YacWhenVoting) {
   EXPECT_CALL(*network, send_vote(_, _)).Times(default_peers.size());
 
   YacHash my_hash("my_proposal_hash", "my_block_hash");
-  yac->vote(my_hash, ClusterOrdering(default_peers));
+
+  auto order = ClusterOrdering::create(default_peers);
+  ASSERT_TRUE(order.has_value());
+
+  yac->vote(my_hash, order.value());
 }
 
 /**